Talip7/scikit-learn-issues-multilabel huggingface dataset:
RFC Future of HalvingGridSearchCV

`HalvingGridSearchCV` has been in experimental mode since its conception in 2020-09, almost 4 years ago.

Things to note:
- we haven't seen many issues regarding these estimators, but that is probably because we are not advertising them enough, and they're in experimental mode.
- an issue such as https://github.com/scikit-learn/scikit-learn/issues/27422 gets very minimal traction and there are not many active maintainers who are actively maintaining that part of the codebase
- the API as is, is confusing and we could work on clearing things up and improving it, which shouldn't be painful since it's still experimental and we don't need to go through deprecation cycles

Now the question is, what do we think about it? Options we have:
- move them out of experimental w/o change: I don't think this is a good idea and we should probably improve things before doing so
- remove from scikit-learn: they are useful estimators, but are they used? Do we think making them more prominent in our documentation would help? Do we want to do that?
- improve the status quo of the estimator, improve documentation, move out of experimental: is this a priority for us? Are people here who are willing to dedicate time to work on it / review the work?

Also, would be nice to see if they're used, and if yes, how. Maybe @amueller or @NicolasHug would have an idea here?

cc @scikit-learn/core-devs

COMMENT:
👍  to improving and/or simplifying instead of removing.

My only experience from using it myself is that indeed it is a bit fiddly to make it do what you think you want it to do/figure out what you want it to do. This was around what the "resource" is, how that will change over the iterations and whether or not that gets me to "the end" I wanted. If this text is a bit confusing, then it is a good reflection of my thinking at the time - a bit confused :D

Maybe a way to reduce the complexity (for users) is to only support `n_samples` as the "resource"? Or maybe an estimator specific resource like `n_iter`. The reason I am thinking that maybe `n_samples` would be enough is that for a lot of estimators there is no good estimator specific resource and/or you can grid search over the value and/or use some form of early stopping. Basically, is it worth being able to set the number of trees in my random forest as resource or is limiting the size of the dataset actually more useful in terms of "saving resources"? My assumption being that you are using halving grid search because you can't afford a full random grid search of your hyper-parameters as it would take too much human time to do so.

This means IMHO the main use-case for these estimators is "I would random search but that takes too long". Is this also what others see as the/a main use-case? If not, what is your _main_ use-case?

Labels: RFC
RFC Future of HalvingGridSearchCV

`HalvingGridSearchCV` has been in experimental mode since its conception in 2020-09, almost 4 years ago.

Things to note:
- we haven't seen many issues regarding these estimators, but that is probably because we are not advertising them enough, and they're in experimental mode.
- an issue such as https://github.com/scikit-learn/scikit-learn/issues/27422 gets very minimal traction and there are not many active maintainers who are actively maintaining that part of the codebase
- the API as is, is confusing and we could work on clearing things up and improving it, which shouldn't be painful since it's still experimental and we don't need to go through deprecation cycles

Now the question is, what do we think about it? Options we have:
- move them out of experimental w/o change: I don't think this is a good idea and we should probably improve things before doing so
- remove from scikit-learn: they are useful estimators, but are they used? Do we think making them more prominent in our documentation would help? Do we want to do that?
- improve the status quo of the estimator, improve documentation, move out of experimental: is this a priority for us? Are people here who are willing to dedicate time to work on it / review the work?

Also, would be nice to see if they're used, and if yes, how. Maybe @amueller or @NicolasHug would have an idea here?

cc @scikit-learn/core-devs

COMMENT:
In the context of HalvingGridSearchCV and the whole Hyperband family of hyper-parameter search techniques, the "resource" is normally an iteration. I recall most of the research is around neural networks where the resource is the number of epochs. Besides neural networks, scikit-learn's HistGradientBoosting's `max_iter` can be considered a "resource". We do not really leverage the full speed up from using `max_iter` as a resource because we do not use warm starting: https://github.com/scikit-learn/scikit-learn/issues/15125

I'll like to keep "epoches" or "iterations" as the resource because this is what paper usually study. Although for scikit-learn, it is harder to configure compare to using the number of samples.

Labels: RFC
DEP loss_function_ attribute in PassiveAggressiveClassifier

#27979 deprecate the attribute `loss_function_` that accesses a Cython extension class in `SGDClassifier` and `SGDOneClassSVM`. Unfortunately, `PassiveAggressiveClassifier` also inherits the `loss_function_` attribute from `BaseSGDClassifier` and this was overlooked in #27979.

Now, what do we do?
1. Proper deprecation cycle delaying #28049 for another year.
2. Remove it with 1.6 anyway.
3. Add the loss function as python function in `PassiveAggressiveClassifier` only, deprecate and remove in 2 releases.

COMMENT:
Given that we do get a warning when accessing the `loss_function_` attribute, because it's done in the base class fortunately, I'd go with `2.`. We should still add the deprecated directives in `PassiveAggressiveClassifier`'s docstring for the time being though.

Labels: API, Needs Decision
DEP loss_function_ attribute in PassiveAggressiveClassifier

#27979 deprecate the attribute `loss_function_` that accesses a Cython extension class in `SGDClassifier` and `SGDOneClassSVM`. Unfortunately, `PassiveAggressiveClassifier` also inherits the `loss_function_` attribute from `BaseSGDClassifier` and this was overlooked in #27979.

Now, what do we do?
1. Proper deprecation cycle delaying #28049 for another year.
2. Remove it with 1.6 anyway.
3. Add the loss function as python function in `PassiveAggressiveClassifier` only, deprecate and remove in 2 releases.

COMMENT:
We can also deprecate and remove the class altogether 😝

Labels: API, Needs Decision
`sklearn.neighbors.NearestNeighbors` allow processing nan values

### Describe the workflow you want to enable

In some cases (for example [memory-based collaborative filtering](https://en.wikipedia.org/wiki/Collaborative_filtering)) empty values is important part of the algorithm. But `sklearn.neighbors.NearestNeighbors` doesn't allow to handle empty values, even if metrics can handle such cases.

The following code snippet shows a simple realisation of building collaboration with `sklearn.neighbors.NearestNeighbors`.
```py
import numpy as np
from sklearn.neighbors import NearestNeighbors
from scipy.spatial.distance import correlation as orig_correlation

def correlation(a,b):
    '''
    Modified correlation function that handles
    nan values. If it's impossible to
    to calculate the distance, it returns 2 - 
    the maximum possible value.
    '''
    cond = ~(np.isnan(a) | np.isnan(b))
    # in case if there are only two
    # observations it's impossible
    # to compute coorrelation coeficient
    # it's invalid case - so we return 
    # the biggest possible distance
    if sum(cond) <=1:
        return 2

    a_std = a[cond].std()
    b_std = b[cond].std()

    # Pearson coefficient uses standard 
    # deviations in the denominator, so 
    # if any of them is equal to zero, 
    # we have to return the biggest
    # possible distance.
    if a_std==0 or b_std==0:
        return 2
    return orig_correlation(a[cond],b[cond])

example_array = np.array(
    [[ 5.,  2.,  4.,  6.,  5.,  4.,  6.,  6.,  7.,  6.],
    [ 6., np.NaN,  4.,  7.,  5.,  4.,  8.,  5.,  7.,  6.],
    [ 7., 10.,  1., np.NaN,  9.,  7., np.NaN,  3., np.NaN,  8.],
    [np.NaN,  2.,  4., np.NaN,  4.,  4.,  7.,  7., np.NaN,  6.],
    [ 8.,  1., np.NaN,  7.,  6.,  2.,  2.,  8.,  2.,  1.],
    [ 8., np.NaN, np.NaN, np.NaN,  8.,  7.,  7.,  4., 10.,  9.],
    [ 8.,  1.,  6.,  8.,  5.,  2.,  2., np.NaN,  3.,  1.],
    [ 6., np.NaN,  0.,  5.,  9.,  7.,  7.,  3.,  9.,  6.],
    [np.NaN,  1.,  7.,  8.,  5.,  2., np.NaN, np.NaN, np.NaN,  1.],
    [ 8.,  1.,  7.,  8.,  5., np.NaN,  2.,  8.,  3.,  1.]]
)

NearestNeighbors(
    metric=correlation
).fit(example_array)
```
It raises `ValueError: Input X contains NaN.`. You can see from the trace that it just happens during the data validation phase - `sklearn/utils/validation.py`, function `_assert_all_finite`.

It would be great if you could use data with empty values in some cases.

### Describe your proposed solution

This can be solved by adding a parameter to `sklearn.neighbors.NearestNeighbors` that allows to use empty values in the algorithm. If this parameter is set to `True`, validation functions won't throw errors if array with nan is passed.

### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
We're happy to allow nans in our estimators, and some work is ongoing in this regard. E.g. #28043 

Feel free to open PR for this.

Labels: New Feature
`sklearn.neighbors.NearestNeighbors` allow processing nan values

### Describe the workflow you want to enable

In some cases (for example [memory-based collaborative filtering](https://en.wikipedia.org/wiki/Collaborative_filtering)) empty values is important part of the algorithm. But `sklearn.neighbors.NearestNeighbors` doesn't allow to handle empty values, even if metrics can handle such cases.

The following code snippet shows a simple realisation of building collaboration with `sklearn.neighbors.NearestNeighbors`.
```py
import numpy as np
from sklearn.neighbors import NearestNeighbors
from scipy.spatial.distance import correlation as orig_correlation

def correlation(a,b):
    '''
    Modified correlation function that handles
    nan values. If it's impossible to
    to calculate the distance, it returns 2 - 
    the maximum possible value.
    '''
    cond = ~(np.isnan(a) | np.isnan(b))
    # in case if there are only two
    # observations it's impossible
    # to compute coorrelation coeficient
    # it's invalid case - so we return 
    # the biggest possible distance
    if sum(cond) <=1:
        return 2

    a_std = a[cond].std()
    b_std = b[cond].std()

    # Pearson coefficient uses standard 
    # deviations in the denominator, so 
    # if any of them is equal to zero, 
    # we have to return the biggest
    # possible distance.
    if a_std==0 or b_std==0:
        return 2
    return orig_correlation(a[cond],b[cond])

example_array = np.array(
    [[ 5.,  2.,  4.,  6.,  5.,  4.,  6.,  6.,  7.,  6.],
    [ 6., np.NaN,  4.,  7.,  5.,  4.,  8.,  5.,  7.,  6.],
    [ 7., 10.,  1., np.NaN,  9.,  7., np.NaN,  3., np.NaN,  8.],
    [np.NaN,  2.,  4., np.NaN,  4.,  4.,  7.,  7., np.NaN,  6.],
    [ 8.,  1., np.NaN,  7.,  6.,  2.,  2.,  8.,  2.,  1.],
    [ 8., np.NaN, np.NaN, np.NaN,  8.,  7.,  7.,  4., 10.,  9.],
    [ 8.,  1.,  6.,  8.,  5.,  2.,  2., np.NaN,  3.,  1.],
    [ 6., np.NaN,  0.,  5.,  9.,  7.,  7.,  3.,  9.,  6.],
    [np.NaN,  1.,  7.,  8.,  5.,  2., np.NaN, np.NaN, np.NaN,  1.],
    [ 8.,  1.,  7.,  8.,  5., np.NaN,  2.,  8.,  3.,  1.]]
)

NearestNeighbors(
    metric=correlation
).fit(example_array)
```
It raises `ValueError: Input X contains NaN.`. You can see from the trace that it just happens during the data validation phase - `sklearn/utils/validation.py`, function `_assert_all_finite`.

It would be great if you could use data with empty values in some cases.

### Describe your proposed solution

This can be solved by adding a parameter to `sklearn.neighbors.NearestNeighbors` that allows to use empty values in the algorithm. If this parameter is set to `True`, validation functions won't throw errors if array with nan is passed.

### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
Not really, that sounds too hackie and I'm not sure there's any theoretical backing for how we'd handle it.

Labels: New Feature
`sklearn.neighbors.NearestNeighbors` allow processing nan values

### Describe the workflow you want to enable

In some cases (for example [memory-based collaborative filtering](https://en.wikipedia.org/wiki/Collaborative_filtering)) empty values is important part of the algorithm. But `sklearn.neighbors.NearestNeighbors` doesn't allow to handle empty values, even if metrics can handle such cases.

The following code snippet shows a simple realisation of building collaboration with `sklearn.neighbors.NearestNeighbors`.
```py
import numpy as np
from sklearn.neighbors import NearestNeighbors
from scipy.spatial.distance import correlation as orig_correlation

def correlation(a,b):
    '''
    Modified correlation function that handles
    nan values. If it's impossible to
    to calculate the distance, it returns 2 - 
    the maximum possible value.
    '''
    cond = ~(np.isnan(a) | np.isnan(b))
    # in case if there are only two
    # observations it's impossible
    # to compute coorrelation coeficient
    # it's invalid case - so we return 
    # the biggest possible distance
    if sum(cond) <=1:
        return 2

    a_std = a[cond].std()
    b_std = b[cond].std()

    # Pearson coefficient uses standard 
    # deviations in the denominator, so 
    # if any of them is equal to zero, 
    # we have to return the biggest
    # possible distance.
    if a_std==0 or b_std==0:
        return 2
    return orig_correlation(a[cond],b[cond])

example_array = np.array(
    [[ 5.,  2.,  4.,  6.,  5.,  4.,  6.,  6.,  7.,  6.],
    [ 6., np.NaN,  4.,  7.,  5.,  4.,  8.,  5.,  7.,  6.],
    [ 7., 10.,  1., np.NaN,  9.,  7., np.NaN,  3., np.NaN,  8.],
    [np.NaN,  2.,  4., np.NaN,  4.,  4.,  7.,  7., np.NaN,  6.],
    [ 8.,  1., np.NaN,  7.,  6.,  2.,  2.,  8.,  2.,  1.],
    [ 8., np.NaN, np.NaN, np.NaN,  8.,  7.,  7.,  4., 10.,  9.],
    [ 8.,  1.,  6.,  8.,  5.,  2.,  2., np.NaN,  3.,  1.],
    [ 6., np.NaN,  0.,  5.,  9.,  7.,  7.,  3.,  9.,  6.],
    [np.NaN,  1.,  7.,  8.,  5.,  2., np.NaN, np.NaN, np.NaN,  1.],
    [ 8.,  1.,  7.,  8.,  5., np.NaN,  2.,  8.,  3.,  1.]]
)

NearestNeighbors(
    metric=correlation
).fit(example_array)
```
It raises `ValueError: Input X contains NaN.`. You can see from the trace that it just happens during the data validation phase - `sklearn/utils/validation.py`, function `_assert_all_finite`.

It would be great if you could use data with empty values in some cases.

### Describe your proposed solution

This can be solved by adding a parameter to `sklearn.neighbors.NearestNeighbors` that allows to use empty values in the algorithm. If this parameter is set to `True`, validation functions won't throw errors if array with nan is passed.

### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
@adrinjalali other than imputation there is not much on missing value in KNN? If you have any thoughts let me know

Labels: New Feature
Samples with nan distance are included in the computation of mean in `KNNImputer` for uniform weights

### Describe the bug

The toy dataset and the distance computed by `nan_euclidean_distances` are as follows.
```python
import numpy as np
from sklearn.metrics.pairwise import nan_euclidean_distances
X_train = [[1, 1], [np.nan, 2]]
X_test = [[0, np.nan]]
print(nan_euclidean_distances(X_test, X_train)) # [[1.41421356, nan]]
```

When `weights` is set to 'uniform', the second sample in `X_train` is _included_. See the code below.
However, when `weights` is set to 'distance', the second sample in `X_train` is _excluded_.

This is because `weight_matrix` where samples with nan distance are set to 0 when `weights` is set to 'distance'.
https://github.com/scikit-learn/scikit-learn/blob/6614f7516a976f0e02bd6587e63f57c712432084/sklearn/impute/_knn.py#L193-L197

To takle this, we could also fill the nans with 0 when `weights` is set to 'uniform'.
```python
if weight_matrix is None:
    weight_matrix = np.ones_like(donors_dist)
    weight_matrix[np.isnan(donors_dist)] = 0.0
```

### Steps/Code to Reproduce

```python
import numpy as np
from sklearn.impute import KNNImputer
X_train = [[1, 1], [np.nan, 2]]
X_test = [[0, np.nan]]

knn_uniform = KNNImputer(n_neighbors=2, weights='uniform').fit(X_train)
print(knn_uniform.transform(X_test))

knn_distance = KNNImputer(n_neighbors=2, weights='distance').fit(X_train)
print(knn_distance.transform(X_test))
```

### Expected Results

```
[[0, 1]] # uniform
[[0, 1]] # distance
```

### Actual Results

```
[[0, 1.5]] # uniform
[[0, 1]]   # distance
```

### Versions

```shell
System:
    python: 3.9.16 | packaged by conda-forge | (main, Feb  1 2023, 21:38:11)  [Clang 14.0.6 ]
executable: /Users/xxf/miniconda3/envs/sklearn-env/bin/python
   machine: macOS-14.5-arm64-arm-64bit

Python dependencies:
      sklearn: 1.6.dev0
          pip: 23.2.1
   setuptools: 68.0.0
        numpy: 1.26.4
        scipy: 1.13.0
       Cython: 3.0.8
       pandas: 2.1.0
   matplotlib: 3.7.2
       joblib: 1.3.0
threadpoolctl: 3.5.0

Built with OpenMP: True

threadpoolctl info:
       user_api: blas
   internal_api: openblas
    num_threads: 8
         prefix: libopenblas
       filepath: /Users/xxf/miniconda3/envs/sklearn-env/lib/libopenblas.0.dylib
        version: 0.3.23
threading_layer: openmp
   architecture: VORTEX

       user_api: openmp
   internal_api: openmp
    num_threads: 8
         prefix: libomp
       filepath: /Users/xxf/miniconda3/envs/sklearn-env/lib/libomp.dylib
        version: None
```

COMMENT:
I think this is legit. WDYT @glemaitre

Labels: Bug
Samples with nan distance are included in the computation of mean in `KNNImputer` for uniform weights

### Describe the bug

The toy dataset and the distance computed by `nan_euclidean_distances` are as follows.
```python
import numpy as np
from sklearn.metrics.pairwise import nan_euclidean_distances
X_train = [[1, 1], [np.nan, 2]]
X_test = [[0, np.nan]]
print(nan_euclidean_distances(X_test, X_train)) # [[1.41421356, nan]]
```

When `weights` is set to 'uniform', the second sample in `X_train` is _included_. See the code below.
However, when `weights` is set to 'distance', the second sample in `X_train` is _excluded_.

This is because `weight_matrix` where samples with nan distance are set to 0 when `weights` is set to 'distance'.
https://github.com/scikit-learn/scikit-learn/blob/6614f7516a976f0e02bd6587e63f57c712432084/sklearn/impute/_knn.py#L193-L197

To takle this, we could also fill the nans with 0 when `weights` is set to 'uniform'.
```python
if weight_matrix is None:
    weight_matrix = np.ones_like(donors_dist)
    weight_matrix[np.isnan(donors_dist)] = 0.0
```

### Steps/Code to Reproduce

```python
import numpy as np
from sklearn.impute import KNNImputer
X_train = [[1, 1], [np.nan, 2]]
X_test = [[0, np.nan]]

knn_uniform = KNNImputer(n_neighbors=2, weights='uniform').fit(X_train)
print(knn_uniform.transform(X_test))

knn_distance = KNNImputer(n_neighbors=2, weights='distance').fit(X_train)
print(knn_distance.transform(X_test))
```

### Expected Results

```
[[0, 1]] # uniform
[[0, 1]] # distance
```

### Actual Results

```
[[0, 1.5]] # uniform
[[0, 1]]   # distance
```

### Versions

```shell
System:
    python: 3.9.16 | packaged by conda-forge | (main, Feb  1 2023, 21:38:11)  [Clang 14.0.6 ]
executable: /Users/xxf/miniconda3/envs/sklearn-env/bin/python
   machine: macOS-14.5-arm64-arm-64bit

Python dependencies:
      sklearn: 1.6.dev0
          pip: 23.2.1
   setuptools: 68.0.0
        numpy: 1.26.4
        scipy: 1.13.0
       Cython: 3.0.8
       pandas: 2.1.0
   matplotlib: 3.7.2
       joblib: 1.3.0
threadpoolctl: 3.5.0

Built with OpenMP: True

threadpoolctl info:
       user_api: blas
   internal_api: openblas
    num_threads: 8
         prefix: libopenblas
       filepath: /Users/xxf/miniconda3/envs/sklearn-env/lib/libopenblas.0.dylib
        version: 0.3.23
threading_layer: openmp
   architecture: VORTEX

       user_api: openmp
   internal_api: openmp
    num_threads: 8
         prefix: libomp
       filepath: /Users/xxf/miniconda3/envs/sklearn-env/lib/libomp.dylib
        version: None
```

COMMENT:
Indeed. It looks like legit. We can review #29135

Labels: Bug
Fix version warning banner on the stable documentation page

### Describe the issue linked to the documentation

Currently https://scikit-learn.org/stable/index.html shows the version warning banner ("This are the docs for an unstable version").
<img width="1258" alt="Screenshot 2024-05-22 at 09 08 02" src="https://github.com/scikit-learn/scikit-learn/assets/1448859/2b1e7309-b11f-4e92-8b80-1a6b96743d9c">

I think this is happening because we haven't updated https://scikit-learn.org/dev/_static/versions.json which lists all the available versions and declares which is the stable version.

This file is generated by `build_tools/circle/list_versions.py` which should run as part of the CI on `main`. My guess as to why the file hasn't been updated is that we have not merged a PR since releasing v1.5.0. If I run the script locally it generates the correct content for the `.json` file. 



### Suggest a potential alternative/fix

I think the fix is to merge any PR into `main` as this will regenerate the `versions.json` file.

This is because all versions of the documentation read the versions from the same URL, which is based on `/dev/`.

COMMENT:
Let's see if merging https://github.com/scikit-learn/scikit-learn/pull/29071 fixed this

Labels: Documentation
Fix version warning banner on the stable documentation page

### Describe the issue linked to the documentation

Currently https://scikit-learn.org/stable/index.html shows the version warning banner ("This are the docs for an unstable version").
<img width="1258" alt="Screenshot 2024-05-22 at 09 08 02" src="https://github.com/scikit-learn/scikit-learn/assets/1448859/2b1e7309-b11f-4e92-8b80-1a6b96743d9c">

I think this is happening because we haven't updated https://scikit-learn.org/dev/_static/versions.json which lists all the available versions and declares which is the stable version.

This file is generated by `build_tools/circle/list_versions.py` which should run as part of the CI on `main`. My guess as to why the file hasn't been updated is that we have not merged a PR since releasing v1.5.0. If I run the script locally it generates the correct content for the `.json` file. 



### Suggest a potential alternative/fix

I think the fix is to merge any PR into `main` as this will regenerate the `versions.json` file.

This is because all versions of the documentation read the versions from the same URL, which is based on `/dev/`.

COMMENT:
Looks like this is now fixed 🎉

Labels: Documentation