Talip7/scikit-learn-issues-multilabel huggingface dataset:
Investigate SAG/SAGA solver

#### Description
The newly introduced tight tests for `Ridge` in #22910 together with the random seed fixture in #22749 revealed some shortcomings of the sag and saga solver:

1. ~~It shows some random behavior even with fixed random seed.~~
2. The `tol` needs to be set much smaller to receive comparable results with the other solvers of `Ridge`.

Ideally, the cause for both issues can be identified and fixed.

#### Some links for context
https://github.com/scikit-learn/scikit-learn/issues/23014
https://github.com/scikit-learn/scikit-learn/pull/23017
https://github.com/scikit-learn/scikit-learn/pull/23026
https://github.com/scikit-learn/scikit-learn/pull/23152
#23177

COMMENT:
Maybe it's related to the scaling of the objective similar to #24752?

Labels: Bug, Moderate, help wanted, module:linear_model
Investigate SAG/SAGA solver

#### Description
The newly introduced tight tests for `Ridge` in #22910 together with the random seed fixture in #22749 revealed some shortcomings of the sag and saga solver:

1. ~~It shows some random behavior even with fixed random seed.~~
2. The `tol` needs to be set much smaller to receive comparable results with the other solvers of `Ridge`.

Ideally, the cause for both issues can be identified and fixed.

#### Some links for context
https://github.com/scikit-learn/scikit-learn/issues/23014
https://github.com/scikit-learn/scikit-learn/pull/23017
https://github.com/scikit-learn/scikit-learn/pull/23026
https://github.com/scikit-learn/scikit-learn/pull/23152
#23177

COMMENT:
I'm interested in investigating this issue. I'll start by looking into why SAG/SAGA requires a much smaller tol in test_ridge.py.

Labels: Bug, Moderate, help wanted, module:linear_model
Wrong normalization for pseudolikelihood in Restricted Boltzmann Machine.

### Describe the bug

Pseudolikelihood of a sample under Restricted Boltzmann machine includes  the number of features  as a multiplier. 
Thus pseudolikelihood reflects the dimensionality of visible units, apart from the sample probability. 
I believe this multiplier should be removed. 

### Steps/Code to Reproduce

from sklearn.neural_network import BernoulliRBM
import numpy as np
 
RBM_= BernoulliRBM(verbose=1) 
X_10= np.ones([10,10])
X_1000= np.ones([10,1000])
                  

print('Learning RBM with 10 visible units:')
RBM_.fit(X_small)
print(100*'_')
print('Learning RBM with 1000 visible units')
RBM_.fit(X_large)
                  

### Expected Results

The pseudolikelihoods for 10 and 1000 visible units should be of the same order at corresponding learning stages.

### Actual Results

Learning RBM with 10 visible units:
[BernoulliRBM] Iteration 1, pseudo-likelihood = -0.19, time = 0.00s
[BernoulliRBM] Iteration 2, pseudo-likelihood = -0.28, time = 0.00s
[BernoulliRBM] Iteration 3, pseudo-likelihood = -0.22, time = 0.00s
[BernoulliRBM] Iteration 4, pseudo-likelihood = -0.12, time = 0.00s
[BernoulliRBM] Iteration 5, pseudo-likelihood = -0.10, time = 0.00s
[BernoulliRBM] Iteration 6, pseudo-likelihood = -0.07, time = 0.00s
[BernoulliRBM] Iteration 7, pseudo-likelihood = -0.07, time = 0.00s
[BernoulliRBM] Iteration 8, pseudo-likelihood = -0.08, time = 0.00s
[BernoulliRBM] Iteration 9, pseudo-likelihood = -0.10, time = 0.01s
[BernoulliRBM] Iteration 10, pseudo-likelihood = -0.06, time = 0.00s
____________________________________________________________________________________________________
Learning RBM with 1000 visible units
[BernoulliRBM] Iteration 1, pseudo-likelihood = -3.80, time = 0.01s
[BernoulliRBM] Iteration 2, pseudo-likelihood = -0.76, time = 0.02s
[BernoulliRBM] Iteration 3, pseudo-likelihood = -0.94, time = 0.01s
[BernoulliRBM] Iteration 4, pseudo-likelihood = -1.97, time = 0.01s
[BernoulliRBM] Iteration 5, pseudo-likelihood = -2.01, time = 0.01s
[BernoulliRBM] Iteration 6, pseudo-likelihood = -0.61, time = 0.01s
[BernoulliRBM] Iteration 7, pseudo-likelihood = -1.06, time = 0.01s
[BernoulliRBM] Iteration 8, pseudo-likelihood = -1.09, time = 0.01s
[BernoulliRBM] Iteration 9, pseudo-likelihood = -0.28, time = 0.01s
[BernoulliRBM] Iteration 10, pseudo-likelihood = -1.01, time = 0.01s


### Versions

```shell
System:
    python: 3.7.3 (default, Mar 27 2019, 22:11:17)  [GCC 7.3.0]
executable: /home/x/PycharmProjects/RBM/env/bin/python
   machine: Linux-4.15.0-142-generic-x86_64-with-debian-stretch-sid

Python dependencies:
          pip: 22.0.4
   setuptools: 40.8.0
      sklearn: 1.0.2
        numpy: 1.21.5
        scipy: 1.7.3
       Cython: None
       pandas: None
   matplotlib: 3.5.1
       joblib: 1.1.0
threadpoolctl: 3.1.0

Built with OpenMP: True
```

COMMENT:
Hi!
I would like to know if I can work on this issue! 
I reproduced the code using the latest version and the bug is still present.

Labels: Bug, module:neural_network, Needs Investigation
roc_auc_score must warn user about using labels instead of probabilities in binary case

### Describe the workflow you want to enable

I've seen a lot of people use `YetAnotherClassifier.predict()` instead of desired `.predict_proba()[:, 1]` and get wrong result. This problem sometimes can be hard to found and IMO it can be very useful to explicitly warn user.

### Describe your proposed solution

Check if provided `y_score` has shape of `(n_samples, )` and that it contains 0 and 1. If so, it should simply warn user about a situation.

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

_No response_

### Additional context

_No response_

COMMENT:
Thank you for opening this issue. I think warning is good overall as long as we do not give warnings for valid input. For example, `DecisionTreeClassifier` can output 0s and 1s in `predict_proba`:

```python
from sklearn.tree import DecisionTreeClassifier
from sklearn.datasets import make_classification

X, y = make_classification(random_state=0)
tree = DecisionTreeClassifier(random_state=0).fit(X, y)
tree.predict_proba(X)[:, 1]
array([0., 0., 1., 1., 1., ... ])
```

To make sure we do not raise a warning for valid input, I think we can raise a warning when `y_score` is a integer and `y_true` is "binary".

Note, I prefer *not* to call `np.unique` on `y_score` to get `(0, 1)` because it adds more overhead.

Labels: New Feature, module:metrics, Needs Decision - Include Feature
roc_auc_score must warn user about using labels instead of probabilities in binary case

### Describe the workflow you want to enable

I've seen a lot of people use `YetAnotherClassifier.predict()` instead of desired `.predict_proba()[:, 1]` and get wrong result. This problem sometimes can be hard to found and IMO it can be very useful to explicitly warn user.

### Describe your proposed solution

Check if provided `y_score` has shape of `(n_samples, )` and that it contains 0 and 1. If so, it should simply warn user about a situation.

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

_No response_

### Additional context

_No response_

COMMENT:
Maybe we could check using the following method instead of `np.unique`
```python
np.alltrue(y_pred.astype(int) == y_pred)
```
which is much faster than using `np.unique` as shown in the following chart:

![image](https://user-images.githubusercontent.com/16646940/164889663-05af4db6-acea-4995-abbd-f48a09c81025.png)

Labels: New Feature, module:metrics, Needs Decision - Include Feature
roc_auc_score must warn user about using labels instead of probabilities in binary case

### Describe the workflow you want to enable

I've seen a lot of people use `YetAnotherClassifier.predict()` instead of desired `.predict_proba()[:, 1]` and get wrong result. This problem sometimes can be hard to found and IMO it can be very useful to explicitly warn user.

### Describe your proposed solution

Check if provided `y_score` has shape of `(n_samples, )` and that it contains 0 and 1. If so, it should simply warn user about a situation.

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

_No response_

### Additional context

_No response_

COMMENT:
For valid input of continuous floats, `y_pred.astype(int)` allocates a new integer array and `y_pred.astype(int) == y_pred` allocates a boolean array. Both arrays are the same size as `y_pred`. I think allocating two arrays is too much memory overhead if we are using it to  check for questionable input.

**Edit**: I guess we can do "if y_score is integer and y_true is binary", then do the expensive operation to check.

Although I can see third party estimators have `decision_function` return integers of `(0, 1)` and the warning would be annoying.

Labels: New Feature, module:metrics, Needs Decision - Include Feature
roc_auc_score must warn user about using labels instead of probabilities in binary case

### Describe the workflow you want to enable

I've seen a lot of people use `YetAnotherClassifier.predict()` instead of desired `.predict_proba()[:, 1]` and get wrong result. This problem sometimes can be hard to found and IMO it can be very useful to explicitly warn user.

### Describe your proposed solution

Check if provided `y_score` has shape of `(n_samples, )` and that it contains 0 and 1. If so, it should simply warn user about a situation.

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

_No response_

### Additional context

_No response_

COMMENT:
If still open, I would like to work on this issue.

Labels: New Feature, module:metrics, Needs Decision - Include Feature
roc_auc_score must warn user about using labels instead of probabilities in binary case

### Describe the workflow you want to enable

I've seen a lot of people use `YetAnotherClassifier.predict()` instead of desired `.predict_proba()[:, 1]` and get wrong result. This problem sometimes can be hard to found and IMO it can be very useful to explicitly warn user.

### Describe your proposed solution

Check if provided `y_score` has shape of `(n_samples, )` and that it contains 0 and 1. If so, it should simply warn user about a situation.

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

_No response_

### Additional context

_No response_

COMMENT:
I do not think there is a clear fix for this issue. If an third party estimator's `decision_function` return integers of `(0, 1)`, it is technically valid input and the warning would not be valid.

As for documentation, we state the functions that are valid inputs for `roc_auc_score`:

https://github.com/scikit-learn/scikit-learn/blob/3275beab03162764463022caa4acafd8a443ac85/sklearn/metrics/_ranking.py#L396-L398

Labels: New Feature, module:metrics, Needs Decision - Include Feature
roc_auc_score must warn user about using labels instead of probabilities in binary case

### Describe the workflow you want to enable

I've seen a lot of people use `YetAnotherClassifier.predict()` instead of desired `.predict_proba()[:, 1]` and get wrong result. This problem sometimes can be hard to found and IMO it can be very useful to explicitly warn user.

### Describe your proposed solution

Check if provided `y_score` has shape of `(n_samples, )` and that it contains 0 and 1. If so, it should simply warn user about a situation.

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

_No response_

### Additional context

_No response_

COMMENT:
`roc_auc_score` function is also used not only for binary-classification metric reports, but also to compare feature vs target values. If we have a feature called "feature_age_column", it should be an integer value. Therefore, no warning should occur if we use it to compare against the target values (i.e. `roc_auc_score(y_true, feature_age_column)`).

Labels: New Feature, module:metrics, Needs Decision - Include Feature
roc_auc_score must warn user about using labels instead of probabilities in binary case

### Describe the workflow you want to enable

I've seen a lot of people use `YetAnotherClassifier.predict()` instead of desired `.predict_proba()[:, 1]` and get wrong result. This problem sometimes can be hard to found and IMO it can be very useful to explicitly warn user.

### Describe your proposed solution

Check if provided `y_score` has shape of `(n_samples, )` and that it contains 0 and 1. If so, it should simply warn user about a situation.

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

_No response_

### Additional context

_No response_

COMMENT:
Cc @StefanieSenger since you were touching on similar topics.

Labels: New Feature, module:metrics, Needs Decision - Include Feature
test_ridge_regression_unpenalized is unstable

see https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=41172&view=logs&jobId=c0f56aa6-924f-525c-3d4d-cdeecf409e66&j=c0f56aa6-924f-525c-3d4d-cdeecf409e66&t=f81f9209-4a58-571a-6d88-f139846a65e1

cc/ @lorentzenchr

COMMENT:
On one side, I feel guilty for all the work I've created by introducing those tests. On the other side, the shortcomings of the solvers become evident and - to formulate positively - it seems that sag and saga have room for improvement.

Labels: Blocker, module:linear_model, module:test-suite
test_ridge_regression_unpenalized is unstable

see https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=41172&view=logs&jobId=c0f56aa6-924f-525c-3d4d-cdeecf409e66&j=c0f56aa6-924f-525c-3d4d-cdeecf409e66&t=f81f9209-4a58-571a-6d88-f139846a65e1

cc/ @lorentzenchr

COMMENT:
What I don't get is that these failures are not deterministic. It's like there's some kind of randomness (but very subtle) with sag and saga. Although for the failure reported here, it happened in 2 CI jobs and the computed coefs are exactly the same in the 2 jobs

Labels: Blocker, module:linear_model, module:test-suite
test_ridge_regression_unpenalized is unstable

see https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=41172&view=logs&jobId=c0f56aa6-924f-525c-3d4d-cdeecf409e66&j=c0f56aa6-924f-525c-3d4d-cdeecf409e66&t=f81f9209-4a58-571a-6d88-f139846a65e1

cc/ @lorentzenchr

COMMENT:
Should we open an issue for this sag/saga?
@jeremiedbb Are you working on this issue or shall I open a PR today evening or tomorrow?

Labels: Blocker, module:linear_model, module:test-suite
test_ridge_regression_unpenalized is unstable

see https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=41172&view=logs&jobId=c0f56aa6-924f-525c-3d4d-cdeecf409e66&j=c0f56aa6-924f-525c-3d4d-cdeecf409e66&t=f81f9209-4a58-571a-6d88-f139846a65e1

cc/ @lorentzenchr

COMMENT:
@lorentzenchr I'm not sure what to do so if you have an idea about how to make it more robust go ahead.

> Should we open an issue for this sag/saga?

Probably. I mean it would be interesting to better understand what we can expect with these 2 solvers

Labels: Blocker, module:linear_model, module:test-suite
test_ridge_regression_unpenalized is unstable

see https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=41172&view=logs&jobId=c0f56aa6-924f-525c-3d4d-cdeecf409e66&j=c0f56aa6-924f-525c-3d4d-cdeecf409e66&t=f81f9209-4a58-571a-6d88-f139846a65e1

cc/ @lorentzenchr

COMMENT:
I probably found the source of the randomness (see #23181). At least now the CI runs should be reproducible :)

Labels: Blocker, module:linear_model, module:test-suite
test_ridge_regression_unpenalized is unstable

see https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=41172&view=logs&jobId=c0f56aa6-924f-525c-3d4d-cdeecf409e66&j=c0f56aa6-924f-525c-3d4d-cdeecf409e66&t=f81f9209-4a58-571a-6d88-f139846a65e1

cc/ @lorentzenchr

COMMENT:
With the randomness of the CI, this failure should not appear anymore. Since the `global_random_seed` fixture has been designed to consider a test passed if it passes for the 0-99 seeds, accepting that failures could happen for some other seed, I think we can consider this a non-issue. Let's close.

Labels: Blocker, module:linear_model, module:test-suite
CalibratedClassifierCV allowing for more types of input data

### Describe the workflow you want to enable

With the current implementation of **CalibratedClassifierCV**, it cannot be used it with generic datasets.
Whenever input data are of mixed type (e.g. Pandas DataFrame), the `_validate_data()` method will fail, due to the default _dtype='numeric'_ condition being imposed.

It would be nice to have a more flexible data validation. Actually, I handle the mixed types inside my base_estimator (a Pipeline).

### Describe your proposed solution

```
X, y = self._validate_data(
                X, y, accept_sparse=['csc', 'csr', 'coo'],
                force_all_finite=False, allow_nd=True
            )
```

could be substituted with
```X, y = self._validate_data(
                X, y, accept_sparse=['csc', 'csr', 'coo'],
                force_all_finite=False, allow_nd=True, **check_params
            )
```

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

_No response_

### Additional context

_No response_

COMMENT:
The call to `_validate_data` is no longer in `CalibrationClassiferCV` on `main` and was released in version 1.0. For reference, it was removed in https://github.com/scikit-learn/scikit-learn/commit/54ff7b7c4f745166258a529c33fec6a5ead0a432. Installing scikit-learn 1.0 should enable the new behavior of not validating `X`.

Labels: New Feature, Needs Triage
CalibratedClassifierCV allowing for more types of input data

### Describe the workflow you want to enable

With the current implementation of **CalibratedClassifierCV**, it cannot be used it with generic datasets.
Whenever input data are of mixed type (e.g. Pandas DataFrame), the `_validate_data()` method will fail, due to the default _dtype='numeric'_ condition being imposed.

It would be nice to have a more flexible data validation. Actually, I handle the mixed types inside my base_estimator (a Pipeline).

### Describe your proposed solution

```
X, y = self._validate_data(
                X, y, accept_sparse=['csc', 'csr', 'coo'],
                force_all_finite=False, allow_nd=True
            )
```

could be substituted with
```X, y = self._validate_data(
                X, y, accept_sparse=['csc', 'csr', 'coo'],
                force_all_finite=False, allow_nd=True, **check_params
            )
```

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

_No response_

### Additional context

_No response_

COMMENT:
Thank you so much @thomasjpfan 
Sorry for wasting your time with my newbie error of not having an up-to-date version

Labels: New Feature, Needs Triage
ndcg_score

### Describe the bug

ndcg_score doesn't work. you're a bunch of useless spaghetti coders, just go to italian restaurant if you like pasta, don't waste other people's time producing tons of govnokod.

### Steps/Code to Reproduce

import ndcg_score

### Expected Results

work

### Actual Results

no work

### Versions

```shell
all
```

COMMENT:
I'll just close this issue.

@irodionzaytsev if you want your issue to ever be considered, first please be respectful in your discussions and then try to provide a reproducible example showing the wrong behavior.

Labels: Bug, Needs Triage
Clarify and test dropping categories in linear models

#### Proposition
- [ ] Improve documentation about dropping or not dropping category levels.
- [ ] Add tests that linear models (with canonical link) satisfy for L2 penalized coefficients of one category: `sum_{levels} coef = 0`

#### Reasoning
We tell 2 different stories, e.g., in https://scikit-learn.org/dev/modules/generated/sklearn.preprocessing.OneHotEncoder.html and other places:

1. Use `OneHotEncoder` and do **not** drop any category level as that would break symmetry for penalized linear models. 
2. Use `drop="if_binary"`.

Those two points seem, at first glance, contradictory.

#### Background
https://doi.org/10.1093/biostatistics/kxm010 in Eq. (3.12) and following text, it becomes clear that summing over the L2 penalized coefficients of OHEded levels of one category (keeping all levels) yields zero.
Thus for binary features, keeping both levels would yield `coef_1 = - coef_2`. So dropping one level is reasonable (but results in a slightly different model if other features are present and if my math is correct).

COMMENT:
@glemaitre This popped up in https://github.com/scikit-learn/scikit-learn/pull/23082#discussion_r846177150. This is also related to #22892.

Labels: Documentation, module:linear_model, module:preprocessing, module:test-suite
Clarify and test dropping categories in linear models

#### Proposition
- [ ] Improve documentation about dropping or not dropping category levels.
- [ ] Add tests that linear models (with canonical link) satisfy for L2 penalized coefficients of one category: `sum_{levels} coef = 0`

#### Reasoning
We tell 2 different stories, e.g., in https://scikit-learn.org/dev/modules/generated/sklearn.preprocessing.OneHotEncoder.html and other places:

1. Use `OneHotEncoder` and do **not** drop any category level as that would break symmetry for penalized linear models. 
2. Use `drop="if_binary"`.

Those two points seem, at first glance, contradictory.

#### Background
https://doi.org/10.1093/biostatistics/kxm010 in Eq. (3.12) and following text, it becomes clear that summing over the L2 penalized coefficients of OHEded levels of one category (keeping all levels) yields zero.
Thus for binary features, keeping both levels would yield `coef_1 = - coef_2`. So dropping one level is reasonable (but results in a slightly different model if other features are present and if my math is correct).

COMMENT:
Hi, I would like to contribute to this issue. This is my first one though, so it might take so time!

Labels: Documentation, module:linear_model, module:preprocessing, module:test-suite
Clarify and test dropping categories in linear models

#### Proposition
- [ ] Improve documentation about dropping or not dropping category levels.
- [ ] Add tests that linear models (with canonical link) satisfy for L2 penalized coefficients of one category: `sum_{levels} coef = 0`

#### Reasoning
We tell 2 different stories, e.g., in https://scikit-learn.org/dev/modules/generated/sklearn.preprocessing.OneHotEncoder.html and other places:

1. Use `OneHotEncoder` and do **not** drop any category level as that would break symmetry for penalized linear models. 
2. Use `drop="if_binary"`.

Those two points seem, at first glance, contradictory.

#### Background
https://doi.org/10.1093/biostatistics/kxm010 in Eq. (3.12) and following text, it becomes clear that summing over the L2 penalized coefficients of OHEded levels of one category (keeping all levels) yields zero.
Thus for binary features, keeping both levels would yield `coef_1 = - coef_2`. So dropping one level is reasonable (but results in a slightly different model if other features are present and if my math is correct).

COMMENT:
@LouisCout There easier issues to start with. You can filter by the tags "good first issue" and "easy".

Labels: Documentation, module:linear_model, module:preprocessing, module:test-suite
⚠️ CI failed on macOS.pylatest_conda_mkl_no_openmp ⚠️

**CI Failed on [macOS.pylatest_conda_mkl_no_openmp](https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=41102&view=logs&j=e6d5b7c0-0dfd-5ddf-13d5-c71bebf56ce2)**
- test_neighbors_metrics[float32-wminkowski]

COMMENT:
Triggered by #23110 that now runs the `global_dtype` fixture on nightly builds.

Caused by #23142.

Labels: Needs Triage