Talip7/scikit-learn-issues-multilabel huggingface dataset:
RFC introduce methods to get and set estimators' state

Right now `clone` uses `{get, set}_params` to replicate an unfit estimator. These methods are designed to return esimators' hyperparameters. At the moment, we have no way of getting the state of a fitted estimator in a non-pickle format.

Pickle files are by design able to run arbitrary code, and therefore one should ideally only load a pickle file from a trusted source. This makes sharing and moving scikit-learn based estimators hard, which also introduces security issues when deploying ML models in production.

Another issue with pickle files is that we kinda force people to use the same versions of the libraries they used to train the model and dump the pickle. This prevents people from being able to update their base docker images when they're deploying a model which was trained a while ago, and I'm not sure if we have good ways of letting them update their pickle files for a new version.

My proposal is to introduce `{get, set}_state` methods on the `BaseEstimator` level be able to persist and set the state of models in a more portable, secure, and backward compatible way. We can probably even just do JSON.

cc @scikit-learn/core-devs @koaning

COMMENT:
> Another use case for sklearn-to-sklearn without arbitrary code injection a-la pickle

I just want to stress that we will not be successful to implement sklearn-to-sklearn mapping in a way that is complete (all features, all models) in a secure way. This will not happen. There are too many places where we have arguments that open the door to injection (n_jobs being a string that get's evaled, all sorts of callable in many objects, scores that are classes).

Labels: API, RFC
RFC introduce methods to get and set estimators' state

Right now `clone` uses `{get, set}_params` to replicate an unfit estimator. These methods are designed to return esimators' hyperparameters. At the moment, we have no way of getting the state of a fitted estimator in a non-pickle format.

Pickle files are by design able to run arbitrary code, and therefore one should ideally only load a pickle file from a trusted source. This makes sharing and moving scikit-learn based estimators hard, which also introduces security issues when deploying ML models in production.

Another issue with pickle files is that we kinda force people to use the same versions of the libraries they used to train the model and dump the pickle. This prevents people from being able to update their base docker images when they're deploying a model which was trained a while ago, and I'm not sure if we have good ways of letting them update their pickle files for a new version.

My proposal is to introduce `{get, set}_state` methods on the `BaseEstimator` level be able to persist and set the state of models in a more portable, secure, and backward compatible way. We can probably even just do JSON.

cc @scikit-learn/core-devs @koaning

COMMENT:
A few thoughts:

* Security: we can choose to reject any where a secure state cannot be encoded, or we can back off to pickle and disable their unpickling without an explicit flag.
* Efficiency: the suggestion of JSON seems reasonable, except that we might want to consider more efficient storage of arrays. This is where we move towards the reinvention of Pickle and its versioning...
* Scope: Do we consider "state" to be all settings that are sufficient for prediction from a model? In this case we would need to store some "params" as state too, if they are used in prediction. Or do we assume that parameters are serialised separately? For warm starting it is less clear what "sufficient" parameters would be.
* Backwards compatibility: It may be worth considering what kinds of changes to models actually break compatibility across versions, and what testing would look like to reduce this risk. Pickle-level incompatibility has been an issue because of our reliance on `__dict__` equality; where a model param is _stored_ in a different attribute or representation, the pickle broke. If we are rather only serialising content without reference to how they are stored in `__dict__` we might add some maintenance burden and limit the efficiency of this serialisation process, but it's possible we can make most implementation changes not affect deserialisation of old stored state.

Labels: API, RFC
RFC introduce methods to get and set estimators' state

Right now `clone` uses `{get, set}_params` to replicate an unfit estimator. These methods are designed to return esimators' hyperparameters. At the moment, we have no way of getting the state of a fitted estimator in a non-pickle format.

Pickle files are by design able to run arbitrary code, and therefore one should ideally only load a pickle file from a trusted source. This makes sharing and moving scikit-learn based estimators hard, which also introduces security issues when deploying ML models in production.

Another issue with pickle files is that we kinda force people to use the same versions of the libraries they used to train the model and dump the pickle. This prevents people from being able to update their base docker images when they're deploying a model which was trained a while ago, and I'm not sure if we have good ways of letting them update their pickle files for a new version.

My proposal is to introduce `{get, set}_state` methods on the `BaseEstimator` level be able to persist and set the state of models in a more portable, secure, and backward compatible way. We can probably even just do JSON.

cc @scikit-learn/core-devs @koaning

COMMENT:
We ended up using `__getstate__`/`__setstate__` used by `pickle`, and aren't going to support cross version load, which simplifies things quite a bit. There are still challenges with certain types, especially C extension types, but that'll be a separate issue.

ref: https://skops.readthedocs.io/en/stable/persistence.html

Labels: API, RFC
can't convert a list to lowercase list

### Describe the bug

```pytb
[sklearn/feature_extraction/text.py]n _preprocess(doc, accent_function, lower)
     69     """
     70     if lower:
---> 71         doc = doc.lower()
     72     if accent_function is not None:
     73         doc = accent_function(doc)

AttributeError: 'list' object has no attribute 'lower'
```

### Steps/Code to Reproduce

```python
def cosine_similarity_T(k, query):
  print("1")
  preprocessed_query = re.sub("\W+", " ", query).strip()
  print("2")
  tokens = word_tokenize(str(preprocessed_query))
  q_df = pd.DataFrame(columns=['q_clean'])
  q_df.loc[0,'q_clean'] =tokens
  q_df['q_clean'] =q_df['q_clean'].apply(lambda x: remove_stopwords(x))
  d_cosines = []
  print("3")
  query_vector = gen_vector_T(q_df['q_clean'])
  
    
  for d in vectors.A:
    d_cosines.append(cosine_sim(query_vector, d))
                    
  out = np.array(d_cosines).argsort()[-k:][::-1]
    #print("")
  d_cosines.sort()
  a = pd.DataFrame()
  for i,index in enumerate(out):
    a.loc[i,'index'] = str(index)
    a.loc[i,'Subject'] = df['Subject'][index]
  for j,simScore in enumerate(d_cosines[-k:][::-1]):
    a.loc[j,'Score'] = simScore
  return 
```

### Expected Results

ist gets converted to lowercase as well

### Actual Results

only string gets converted to lowecase

### Versions

```shell
wish we could have that functionality to convert a list to lowercase too as we do for a string
```

COMMENT:
I see you posted code snippets of what you are trying to do. But for us to debug this issue, we still need a [minimal bug report](https://matthewrocklin.com/blog/work/2018/02/28/minimal-bug-reports).

Labels: Bug, Needs Reproducible Code
can't convert a list to lowercase list

### Describe the bug

```pytb
[sklearn/feature_extraction/text.py]n _preprocess(doc, accent_function, lower)
     69     """
     70     if lower:
---> 71         doc = doc.lower()
     72     if accent_function is not None:
     73         doc = accent_function(doc)

AttributeError: 'list' object has no attribute 'lower'
```

### Steps/Code to Reproduce

```python
def cosine_similarity_T(k, query):
  print("1")
  preprocessed_query = re.sub("\W+", " ", query).strip()
  print("2")
  tokens = word_tokenize(str(preprocessed_query))
  q_df = pd.DataFrame(columns=['q_clean'])
  q_df.loc[0,'q_clean'] =tokens
  q_df['q_clean'] =q_df['q_clean'].apply(lambda x: remove_stopwords(x))
  d_cosines = []
  print("3")
  query_vector = gen_vector_T(q_df['q_clean'])
  
    
  for d in vectors.A:
    d_cosines.append(cosine_sim(query_vector, d))
                    
  out = np.array(d_cosines).argsort()[-k:][::-1]
    #print("")
  d_cosines.sort()
  a = pd.DataFrame()
  for i,index in enumerate(out):
    a.loc[i,'index'] = str(index)
    a.loc[i,'Subject'] = df['Subject'][index]
  for j,simScore in enumerate(d_cosines[-k:][::-1]):
    a.loc[j,'Score'] = simScore
  return 
```

### Expected Results

ist gets converted to lowercase as well

### Actual Results

only string gets converted to lowecase

### Versions

```shell
wish we could have that functionality to convert a list to lowercase too as we do for a string
```

COMMENT:
It looks like the vectorizer was inputted a list of list of strings or a DataFrame with strings, which the vectorizer does not support. A workaround is to use pass in the pandas Series instead of the DataFrame.

I am closing this issue as there is no response and this is likely a duplicate of https://github.com/scikit-learn/scikit-learn/issues/16972.

Labels: Bug, Needs Reproducible Code
Symmetric Mean Absolute Percentage Error

### Describe the workflow you want to enable

Make Symmetric Mean Absolute Percentage Error available as an error metric.

### Describe your proposed solution

implement as a metric under _regresion.py

    smape = np.abs(y_pred - y_true) / np.maximum((np.abs(y_true) + np.abs(y_true))/2, epsilon)


### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
It is something that I was proposing there: https://github.com/scikit-learn/scikit-learn/issues/20907#issuecomment-911563939

We could make it an option to the current function most probably.

Labels: New Feature
Symmetric Mean Absolute Percentage Error

### Describe the workflow you want to enable

Make Symmetric Mean Absolute Percentage Error available as an error metric.

### Describe your proposed solution

implement as a metric under _regresion.py

    smape = np.abs(y_pred - y_true) / np.maximum((np.abs(y_true) + np.abs(y_true))/2, epsilon)


### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
Sounds like a good idea to have it as an default off option.

Labels: New Feature
Symmetric Mean Absolute Percentage Error

### Describe the workflow you want to enable

Make Symmetric Mean Absolute Percentage Error available as an error metric.

### Describe your proposed solution

implement as a metric under _regresion.py

    smape = np.abs(y_pred - y_true) / np.maximum((np.abs(y_true) + np.abs(y_true))/2, epsilon)


### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
@Klas96 May I ask for your use case? Why should this metric be in scikit-learn?

Labels: New Feature
Symmetric Mean Absolute Percentage Error

### Describe the workflow you want to enable

Make Symmetric Mean Absolute Percentage Error available as an error metric.

### Describe your proposed solution

implement as a metric under _regresion.py

    smape = np.abs(y_pred - y_true) / np.maximum((np.abs(y_true) + np.abs(y_true))/2, epsilon)


### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
@lorentzenchr ofcourse, this is a handy metric bounded by 0 and 1 that does not become very large when y_true = 0 and y_pred != 0.

Labels: New Feature
Symmetric Mean Absolute Percentage Error

### Describe the workflow you want to enable

Make Symmetric Mean Absolute Percentage Error available as an error metric.

### Describe your proposed solution

implement as a metric under _regresion.py

    smape = np.abs(y_pred - y_true) / np.maximum((np.abs(y_true) + np.abs(y_true))/2, epsilon)


### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
Could you be more specific? Your use case if possible. Is it used elsewhere? Literature? Do you have an idea what an "ideal model" wrt this metric would predict? (Eg. the conditional expectation of y given X is ideal for the squared error.)

Labels: New Feature
Symmetric Mean Absolute Percentage Error

### Describe the workflow you want to enable

Make Symmetric Mean Absolute Percentage Error available as an error metric.

### Describe your proposed solution

implement as a metric under _regresion.py

    smape = np.abs(y_pred - y_true) / np.maximum((np.abs(y_true) + np.abs(y_true))/2, epsilon)


### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
Closing as discussion seems to have stopped. Also, I'm leaning towards -1 for inclusion.

Labels: New Feature
Symmetric Mean Absolute Percentage Error

### Describe the workflow you want to enable

Make Symmetric Mean Absolute Percentage Error available as an error metric.

### Describe your proposed solution

implement as a metric under _regresion.py

    smape = np.abs(y_pred - y_true) / np.maximum((np.abs(y_true) + np.abs(y_true))/2, epsilon)


### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
@lorentzenchr - I would add a vote in favour of adding some MAPE variants / alternatives to the metrics package (I'm happy to assist in preparing a PR).

Classic MAPE suffers from a number of issues, one of the most severe being that as `y_true` -> 0, MAPE -> +inf. Even if a single value in the series is at/close to 0, the entire average can be thrown off by such a large outlier.

Other metrics such as SMAPE and MASE (amongst others) have been developed to help deal with these problems. It would be helpful to provide implementations for some of these.

Here are a couple references to consult:
https://en.wikipedia.org/wiki/Mean_absolute_percentage_error#Issues
https://otexts.com/fpp3/accuracy.html#percentage-errors

Labels: New Feature
Symmetric Mean Absolute Percentage Error

### Describe the workflow you want to enable

Make Symmetric Mean Absolute Percentage Error available as an error metric.

### Describe your proposed solution

implement as a metric under _regresion.py

    smape = np.abs(y_pred - y_true) / np.maximum((np.abs(y_true) + np.abs(y_true))/2, epsilon)


### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
I second what @tnixon said, and think alternatives to MAPE should be included in sklearn, such as SMAPE.

Labels: New Feature
Symmetric Mean Absolute Percentage Error

### Describe the workflow you want to enable

Make Symmetric Mean Absolute Percentage Error available as an error metric.

### Describe your proposed solution

implement as a metric under _regresion.py

    smape = np.abs(y_pred - y_true) / np.maximum((np.abs(y_true) + np.abs(y_true))/2, epsilon)


### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
@tnixon @lorentzenchr Has there been any development on the SMAPE front since the last discussion? I would also find it helpful and I can also help with the implementation if no resources are available from your side.

Labels: New Feature
Symmetric Mean Absolute Percentage Error

### Describe the workflow you want to enable

Make Symmetric Mean Absolute Percentage Error available as an error metric.

### Describe your proposed solution

implement as a metric under _regresion.py

    smape = np.abs(y_pred - y_true) / np.maximum((np.abs(y_true) + np.abs(y_true))/2, epsilon)


### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
A challenge for those asking for symmetric MAPE: Say target $Y\sim F$ with some probability distribution $F$. If you can tell me, what property of $F$ is predicted when minimizing symmetric MAPE (Bayes' rule), then I consider my vote again.

Also note that Rob Hyndman advices against sMAPE, see https://otexts.com/fpp3/accuracy.html#percentage-errors and https://doi.org/10.1016/j.ijforecast.2006.03.001 .

Labels: New Feature
[MAINT] Modularize Tree code and Splitter utility functions

From #20819 , developers expressed issues with the current tree code.

Part of that is the modularity and as a result, maintainability/upgradability of such code. I propose the following super-short refactors to the `_tree.pyx/pxd` and `_splitter.pyx/pxd` files. This would be the first in a series of PRs to demonstrate that #20819 is fairly straightforward.

### Tree class

The Tree class assumes axis-aligned splits. **However, by modularizing the parts where the node values are set, and the feature values are computed for any given dataset, then any subclass of Tree can easily redefine only these two functions and a new `Splitter` to enable a "new" type of Tree.**

I propose adding the following two functions to the `Tree` class and altering `_add_node()`, `_apply_dense` to accompany these changes:

```cython
    cdef int _set_node_values(self, SplitRecord split_node,
            Node *node) nogil except -1:
        """Set node data.
        """
        node.feature = split_node.feature
        node.threshold = split_node.threshold
        return 1
    
  cdef DTYPE_t _compute_feature(self, const DTYPE_t[:] X_ndarray,
            Node *node) nogil:
        """Compute feature from a given data matrix, X.

        In axis-aligned trees, this is simply the value in the column of X
        for this specific feature.
        """
        # the feature index
        cdef DTYPE_t feature = X_ndarray[node.feature]
        return feature
```



### Splitter

Splitter uses functions only defined in the `.pyx` files. As a result, they are not available via `cimport`. **This poses an issue for #20819 and also for downstream packages that might want to define a new splitter that subclasses `Splitter`.** 

Here I propose adding the following functions into the `_splitter.pxd` file:

```
cdef inline void sort(DTYPE_t* Xf, SIZE_t* samples, SIZE_t n) nogil
cdef inline void swap(DTYPE_t* Xf, SIZE_t* samples, SIZE_t i, SIZE_t j) nogil
# and the other splitter utility functions.
...
`

### Misc Notes

This specifically addresses only issues with dense arrays. A follow-on issue and PR would be necessary for sparse arrays.

COMMENT:
The functions in `_splitter.px` may change and we can not guarantee that they are stable.

As part of my initiative to reduce the code in `trees`, (https://github.com/scikit-learn/scikit-learn/pull/22630 or https://github.com/scikit-learn/scikit-learn/pull/22328) functions are being removed. A concrete example is `_splitter`'s `sort` which may be removed for [c++ sort](https://en.cppreference.com/w/cpp/algorithm/sort). If external libraries want to use those functions, the safest thing to do is to vendor them.

Labels: module:tree, Refactor
[MAINT] Modularize Tree code and Splitter utility functions

From #20819 , developers expressed issues with the current tree code.

Part of that is the modularity and as a result, maintainability/upgradability of such code. I propose the following super-short refactors to the `_tree.pyx/pxd` and `_splitter.pyx/pxd` files. This would be the first in a series of PRs to demonstrate that #20819 is fairly straightforward.

### Tree class

The Tree class assumes axis-aligned splits. **However, by modularizing the parts where the node values are set, and the feature values are computed for any given dataset, then any subclass of Tree can easily redefine only these two functions and a new `Splitter` to enable a "new" type of Tree.**

I propose adding the following two functions to the `Tree` class and altering `_add_node()`, `_apply_dense` to accompany these changes:

```cython
    cdef int _set_node_values(self, SplitRecord split_node,
            Node *node) nogil except -1:
        """Set node data.
        """
        node.feature = split_node.feature
        node.threshold = split_node.threshold
        return 1
    
  cdef DTYPE_t _compute_feature(self, const DTYPE_t[:] X_ndarray,
            Node *node) nogil:
        """Compute feature from a given data matrix, X.

        In axis-aligned trees, this is simply the value in the column of X
        for this specific feature.
        """
        # the feature index
        cdef DTYPE_t feature = X_ndarray[node.feature]
        return feature
```



### Splitter

Splitter uses functions only defined in the `.pyx` files. As a result, they are not available via `cimport`. **This poses an issue for #20819 and also for downstream packages that might want to define a new splitter that subclasses `Splitter`.** 

Here I propose adding the following functions into the `_splitter.pxd` file:

```
cdef inline void sort(DTYPE_t* Xf, SIZE_t* samples, SIZE_t n) nogil
cdef inline void swap(DTYPE_t* Xf, SIZE_t* samples, SIZE_t i, SIZE_t j) nogil
# and the other splitter utility functions.
...
`

### Misc Notes

This specifically addresses only issues with dense arrays. A follow-on issue and PR would be necessary for sparse arrays.

COMMENT:
> A concrete example _splitter's sort which may be removed for [c++ sort](https://en.cppreference.com/w/cpp/algorithm/sort).

I agree, and I would be in favour of making use libcpp as much as possible.

This can be through renaming and extending [`sklearn.neighbors._partition_nodes`](https://github.com/scikit-learn/scikit-learn/blob/main/sklearn/neighbors/_partition_nodes.pyx) to define new Cython interfaces to C++ routines.

Labels: module:tree, Refactor