Talip7/scikit-learn-issues-multilabel huggingface dataset:
RFC deprecate the SAMME.R algorithm in AdaBoostClassifier

While working on the following example (https://github.com/scikit-learn/scikit-learn/pull/26486), I found a couple of issues regarding the SAMME.R algorithm, which is the default algorithm in `AdaBoostClassifier`:

- The algorithm was implemented based on the [following paper](https://hastie.su.domains/Papers/samme.pdf). However, this paper is a preprint. In the [final version of the paper](https://www.intlpress.com/site/pub/files/_fulltext/journals/sii/2009/0002/0003/SII-2009-0002-0003-a008.pdf), the SAMME.R algorithm is not presented. So we implemented an unpublished algorithm.
- SAMME.R can show some diverging behaviour as shown in https://github.com/scikit-learn/scikit-learn/issues/20443#issuecomment-1574024873.

For these two reasons, I think that we should deprecate this algorithm and remove the parameter `algorithm` from the `AdaBoostClassifier`.

In addition, I think that we should monitor the latest work on multiclass AdaBoost, where additional theoretical founding is revealed, cf. https://proceedings.neurips.cc/paper/2021/hash/17f5e6db87929fb55cebeb7fd58c1d41-Abstract.html

COMMENT:
For the record, I am +1 on deprecating `SAMME.R` in `AdaBoostClassifer`.

Labels: API, RFC, module:ensemble
RFC deprecate the SAMME.R algorithm in AdaBoostClassifier

While working on the following example (https://github.com/scikit-learn/scikit-learn/pull/26486), I found a couple of issues regarding the SAMME.R algorithm, which is the default algorithm in `AdaBoostClassifier`:

- The algorithm was implemented based on the [following paper](https://hastie.su.domains/Papers/samme.pdf). However, this paper is a preprint. In the [final version of the paper](https://www.intlpress.com/site/pub/files/_fulltext/journals/sii/2009/0002/0003/SII-2009-0002-0003-a008.pdf), the SAMME.R algorithm is not presented. So we implemented an unpublished algorithm.
- SAMME.R can show some diverging behaviour as shown in https://github.com/scikit-learn/scikit-learn/issues/20443#issuecomment-1574024873.

For these two reasons, I think that we should deprecate this algorithm and remove the parameter `algorithm` from the `AdaBoostClassifier`.

In addition, I think that we should monitor the latest work on multiclass AdaBoost, where additional theoretical founding is revealed, cf. https://proceedings.neurips.cc/paper/2021/hash/17f5e6db87929fb55cebeb7fd58c1d41-Abstract.html

COMMENT:
So there's an with this deprecation which seems to be a two step deprecation really. One way to do this is:

- deprecated `"SAMME.R"` as an option for `algorithm` in version `x`
	- warning is silenced by setting `algorithm="SAMME"`
- remove `"SAMME.R"` in version `x+2`, and deprecated `algorithm` as an argument
	- warning is silenced by not passing anything to `algorithm`
- remove `algorithm` in version `x+4`
	- no warnings since we've been telling people to not use `algorithm`

Another alternative is to deprecate `"SAMME.R"` and remove `algorithm` together at version `x`, but this I'm not sure how to deal with warnings in this case:
- we need to warn users that we're removing `"SAMME.R"`, and they need to have a way to silence this warning by setting `algorithm` to something (`None`, or `"SAMME"`)
- we also need to warn users that `algorithm` is being removed and that they shouldn't set it.

The above two warnings don't really work together, I'm not sure how to proceed then.

Labels: API, RFC, module:ensemble
RFC deprecate the SAMME.R algorithm in AdaBoostClassifier

While working on the following example (https://github.com/scikit-learn/scikit-learn/pull/26486), I found a couple of issues regarding the SAMME.R algorithm, which is the default algorithm in `AdaBoostClassifier`:

- The algorithm was implemented based on the [following paper](https://hastie.su.domains/Papers/samme.pdf). However, this paper is a preprint. In the [final version of the paper](https://www.intlpress.com/site/pub/files/_fulltext/journals/sii/2009/0002/0003/SII-2009-0002-0003-a008.pdf), the SAMME.R algorithm is not presented. So we implemented an unpublished algorithm.
- SAMME.R can show some diverging behaviour as shown in https://github.com/scikit-learn/scikit-learn/issues/20443#issuecomment-1574024873.

For these two reasons, I think that we should deprecate this algorithm and remove the parameter `algorithm` from the `AdaBoostClassifier`.

In addition, I think that we should monitor the latest work on multiclass AdaBoost, where additional theoretical founding is revealed, cf. https://proceedings.neurips.cc/paper/2021/hash/17f5e6db87929fb55cebeb7fd58c1d41-Abstract.html

COMMENT:
I think we need to go to the first scheme. I don't think we need to hurry by deprecating `algorithm` right away. We might want to look at the literature regarding the multiclass approach.

The first 2 cycles will make use buy time.

Labels: API, RFC, module:ensemble
RFC deprecate the SAMME.R algorithm in AdaBoostClassifier

While working on the following example (https://github.com/scikit-learn/scikit-learn/pull/26486), I found a couple of issues regarding the SAMME.R algorithm, which is the default algorithm in `AdaBoostClassifier`:

- The algorithm was implemented based on the [following paper](https://hastie.su.domains/Papers/samme.pdf). However, this paper is a preprint. In the [final version of the paper](https://www.intlpress.com/site/pub/files/_fulltext/journals/sii/2009/0002/0003/SII-2009-0002-0003-a008.pdf), the SAMME.R algorithm is not presented. So we implemented an unpublished algorithm.
- SAMME.R can show some diverging behaviour as shown in https://github.com/scikit-learn/scikit-learn/issues/20443#issuecomment-1574024873.

For these two reasons, I think that we should deprecate this algorithm and remove the parameter `algorithm` from the `AdaBoostClassifier`.

In addition, I think that we should monitor the latest work on multiclass AdaBoost, where additional theoretical founding is revealed, cf. https://proceedings.neurips.cc/paper/2021/hash/17f5e6db87929fb55cebeb7fd58c1d41-Abstract.html

COMMENT:
I was trying to remove the whole `algorithm` param at once, so then the user would be shown a warning however their behaviour (not specifying algorithm, since the default would change two versions ahead, or specifying algorithm, because they used a param that's going to deprecated. 
Would it work if I'd set algorithm as 'deprecated' as a default in the init() and then redirect it to use samme.r until it's removed in x+2?

Labels: API, RFC, module:ensemble
RFC deprecate the SAMME.R algorithm in AdaBoostClassifier

While working on the following example (https://github.com/scikit-learn/scikit-learn/pull/26486), I found a couple of issues regarding the SAMME.R algorithm, which is the default algorithm in `AdaBoostClassifier`:

- The algorithm was implemented based on the [following paper](https://hastie.su.domains/Papers/samme.pdf). However, this paper is a preprint. In the [final version of the paper](https://www.intlpress.com/site/pub/files/_fulltext/journals/sii/2009/0002/0003/SII-2009-0002-0003-a008.pdf), the SAMME.R algorithm is not presented. So we implemented an unpublished algorithm.
- SAMME.R can show some diverging behaviour as shown in https://github.com/scikit-learn/scikit-learn/issues/20443#issuecomment-1574024873.

For these two reasons, I think that we should deprecate this algorithm and remove the parameter `algorithm` from the `AdaBoostClassifier`.

In addition, I think that we should monitor the latest work on multiclass AdaBoost, where additional theoretical founding is revealed, cf. https://proceedings.neurips.cc/paper/2021/hash/17f5e6db87929fb55cebeb7fd58c1d41-Abstract.html

COMMENT:
Or, I can also implement the first version (only deprecating `SAMME.R`).

Labels: API, RFC, module:ensemble
RFC deprecate the SAMME.R algorithm in AdaBoostClassifier

While working on the following example (https://github.com/scikit-learn/scikit-learn/pull/26486), I found a couple of issues regarding the SAMME.R algorithm, which is the default algorithm in `AdaBoostClassifier`:

- The algorithm was implemented based on the [following paper](https://hastie.su.domains/Papers/samme.pdf). However, this paper is a preprint. In the [final version of the paper](https://www.intlpress.com/site/pub/files/_fulltext/journals/sii/2009/0002/0003/SII-2009-0002-0003-a008.pdf), the SAMME.R algorithm is not presented. So we implemented an unpublished algorithm.
- SAMME.R can show some diverging behaviour as shown in https://github.com/scikit-learn/scikit-learn/issues/20443#issuecomment-1574024873.

For these two reasons, I think that we should deprecate this algorithm and remove the parameter `algorithm` from the `AdaBoostClassifier`.

In addition, I think that we should monitor the latest work on multiclass AdaBoost, where additional theoretical founding is revealed, cf. https://proceedings.neurips.cc/paper/2021/hash/17f5e6db87929fb55cebeb7fd58c1d41-Abstract.html

COMMENT:
The first version will make it easier.

Labels: API, RFC, module:ensemble
RFC deprecate the SAMME.R algorithm in AdaBoostClassifier

While working on the following example (https://github.com/scikit-learn/scikit-learn/pull/26486), I found a couple of issues regarding the SAMME.R algorithm, which is the default algorithm in `AdaBoostClassifier`:

- The algorithm was implemented based on the [following paper](https://hastie.su.domains/Papers/samme.pdf). However, this paper is a preprint. In the [final version of the paper](https://www.intlpress.com/site/pub/files/_fulltext/journals/sii/2009/0002/0003/SII-2009-0002-0003-a008.pdf), the SAMME.R algorithm is not presented. So we implemented an unpublished algorithm.
- SAMME.R can show some diverging behaviour as shown in https://github.com/scikit-learn/scikit-learn/issues/20443#issuecomment-1574024873.

For these two reasons, I think that we should deprecate this algorithm and remove the parameter `algorithm` from the `AdaBoostClassifier`.

In addition, I think that we should monitor the latest work on multiclass AdaBoost, where additional theoretical founding is revealed, cf. https://proceedings.neurips.cc/paper/2021/hash/17f5e6db87929fb55cebeb7fd58c1d41-Abstract.html

COMMENT:
I have a forcasting model that used the previous version of AdaBoostClassifier ("SAMME.R") and was very good at predicting. Now with the SAMME algorithm, the predictions are very poor. Can we keep the "SAMME.R" algorithm as an option ?

Labels: API, RFC, module:ensemble
RFC deprecate the SAMME.R algorithm in AdaBoostClassifier

While working on the following example (https://github.com/scikit-learn/scikit-learn/pull/26486), I found a couple of issues regarding the SAMME.R algorithm, which is the default algorithm in `AdaBoostClassifier`:

- The algorithm was implemented based on the [following paper](https://hastie.su.domains/Papers/samme.pdf). However, this paper is a preprint. In the [final version of the paper](https://www.intlpress.com/site/pub/files/_fulltext/journals/sii/2009/0002/0003/SII-2009-0002-0003-a008.pdf), the SAMME.R algorithm is not presented. So we implemented an unpublished algorithm.
- SAMME.R can show some diverging behaviour as shown in https://github.com/scikit-learn/scikit-learn/issues/20443#issuecomment-1574024873.

For these two reasons, I think that we should deprecate this algorithm and remove the parameter `algorithm` from the `AdaBoostClassifier`.

In addition, I think that we should monitor the latest work on multiclass AdaBoost, where additional theoretical founding is revealed, cf. https://proceedings.neurips.cc/paper/2021/hash/17f5e6db87929fb55cebeb7fd58c1d41-Abstract.html

COMMENT:
Hi @BRBroadgate, thanks for reporting, but what you wrote doesn't help deciding on that. You would need to tell us more about your use case and some code example that shows the drop in prediction accuracy.

The reason we deprecated was that SAMME.R was only a pre-print, if I recall correctly, and did not fulfill the criteria to be included in scikit-learn. So your proposal would come down to a feature-request. For this ideally, you would open an issue. Please be aware that #32660 applies for this kind of request and it might take months before someone looks at this issue, if at all. There is not enough time for maintainers to do the careful check that would be required unfortunately.

Labels: API, RFC, module:ensemble
Handling NaNs in VotingClassifier

### Discussed in https://github.com/scikit-learn/scikit-learn/discussions/26740

<div type='discussions-op-text'>

<sup>Originally posted by **alrichardbollans** June 30, 2023</sup>
I am using a soft voting classifier to combine the output of two base estimators. One of these base estimators (A) sometimes outputs NaN predictions and for these cases the voting classifier provides the prediction given by the other estimator (B), as expected, however the value of `predict_proba` is NaN for the voting classifier where one would expect it to be the value of `predict_proba` given by B.</div>

I think it'd make sense to either ignore NaNs in averaging, or have a constructor argument which would make the averaging here ignore NaNs.

COMMENT:
> One of these base estimators (A) sometimes outputs NaN predictions and for these cases

I do not think that `predict_proba` returning NaNs is not part of the API. Our metrics and estimators does not really consider NaNs in `predict_proba` and assume they are non-NaN probabilities.

Currently, I am -1 on having `VotingClassifier` support NaNs from `predict_proba`.

Labels: module:ensemble, Needs Decision - Include Feature
Handling NaNs in VotingClassifier

### Discussed in https://github.com/scikit-learn/scikit-learn/discussions/26740

<div type='discussions-op-text'>

<sup>Originally posted by **alrichardbollans** June 30, 2023</sup>
I am using a soft voting classifier to combine the output of two base estimators. One of these base estimators (A) sometimes outputs NaN predictions and for these cases the voting classifier provides the prediction given by the other estimator (B), as expected, however the value of `predict_proba` is NaN for the voting classifier where one would expect it to be the value of `predict_proba` given by B.</div>

I think it'd make sense to either ignore NaNs in averaging, or have a constructor argument which would make the averaging here ignore NaNs.

COMMENT:
How do you expect a model to report that it's not confident about an input? I think it makes sense to be able to have a model which basically says "I don't know"

Labels: module:ensemble, Needs Decision - Include Feature
Handling NaNs in VotingClassifier

### Discussed in https://github.com/scikit-learn/scikit-learn/discussions/26740

<div type='discussions-op-text'>

<sup>Originally posted by **alrichardbollans** June 30, 2023</sup>
I am using a soft voting classifier to combine the output of two base estimators. One of these base estimators (A) sometimes outputs NaN predictions and for these cases the voting classifier provides the prediction given by the other estimator (B), as expected, however the value of `predict_proba` is NaN for the voting classifier where one would expect it to be the value of `predict_proba` given by B.</div>

I think it'd make sense to either ignore NaNs in averaging, or have a constructor argument which would make the averaging here ignore NaNs.

COMMENT:
> I do not think that `predict_proba` returning NaNs is not part of the API. Our metrics and estimators does not really consider NaNs in `predict_proba` and assume they are non-NaN probabilities.

You're correct, that the base estimator  (A) returning NaNs is not part of the sklearn API. It is working under a slightly different paradigm and relies on different data than the other base estimators, which means that sometimes it just doesn't know anything about some samples even though the other (sklearn) base estimators may provide some estimate.

Labels: module:ensemble, Needs Decision - Include Feature
Handling NaNs in VotingClassifier

### Discussed in https://github.com/scikit-learn/scikit-learn/discussions/26740

<div type='discussions-op-text'>

<sup>Originally posted by **alrichardbollans** June 30, 2023</sup>
I am using a soft voting classifier to combine the output of two base estimators. One of these base estimators (A) sometimes outputs NaN predictions and for these cases the voting classifier provides the prediction given by the other estimator (B), as expected, however the value of `predict_proba` is NaN for the voting classifier where one would expect it to be the value of `predict_proba` given by B.</div>

I think it'd make sense to either ignore NaNs in averaging, or have a constructor argument which would make the averaging here ignore NaNs.

COMMENT:
> How do you expect a model to report that it's not confident about an input? I think it makes sense to be able to have a model which basically says "I don't know"

I'll say to return probabilities that maximizes entropy. Concretely, `predict_proba` returns a `1/n_classes` for all classes.

If you are thinking outliers, then it's up to the estimator. But I still think `predict_proba` should return non-NaN values.

Labels: module:ensemble, Needs Decision - Include Feature
Handling NaNs in VotingClassifier

### Discussed in https://github.com/scikit-learn/scikit-learn/discussions/26740

<div type='discussions-op-text'>

<sup>Originally posted by **alrichardbollans** June 30, 2023</sup>
I am using a soft voting classifier to combine the output of two base estimators. One of these base estimators (A) sometimes outputs NaN predictions and for these cases the voting classifier provides the prediction given by the other estimator (B), as expected, however the value of `predict_proba` is NaN for the voting classifier where one would expect it to be the value of `predict_proba` given by B.</div>

I think it'd make sense to either ignore NaNs in averaging, or have a constructor argument which would make the averaging here ignore NaNs.

COMMENT:
That makes sense. So I'll close the issue.

Labels: module:ensemble, Needs Decision - Include Feature
⚠️ CI failed on linux_arm64_wheel ⚠️

**CI failed on [linux_arm64_wheel](https://cirrus-ci.com/build/5513128176254976)** (Jul 06, 2023)

COMMENT:
## CI is no longer failing! ✅

[Successful run](https://cirrus-ci.com/build/5513128176254976) on Jul 06, 2023

Labels: Needs Triage
⚠️ CI failed on linux_arm64_wheel ⚠️

**CI failed on [linux_arm64_wheel](https://cirrus-ci.com/build/5513128176254976)** (Jul 06, 2023)

COMMENT:
Actually it looks like a recurring thing. Let keep this one closed in favor of https://github.com/scikit-learn/scikit-learn/issues/26745

Labels: Needs Triage
Document the class DistanceMetric

Right now we don't have any documentation for the public class `DistanceMetric`.
It would be nice to have a description.

xref: https://scikit-learn.org/dev/modules/generated/sklearn.metrics.DistanceMetric.html#sklearn.metrics.DistanceMetric

COMMENT:
So the documentation should include:
- Introduction
- How to create a `DistanceMetric` Object.
- How to compute distances
- and other supported distance metrics

If yes, then can I take this issue?

Labels: Documentation
Document the class DistanceMetric

Right now we don't have any documentation for the public class `DistanceMetric`.
It would be nice to have a description.

xref: https://scikit-learn.org/dev/modules/generated/sklearn.metrics.DistanceMetric.html#sklearn.metrics.DistanceMetric

COMMENT:
We're talking here about a docstring, for example what you see around here:

https://github.com/scikit-learn/scikit-learn/blob/e30a15008942e9e33114202a4660ca7b0ae90e2d/sklearn/cluster/_agglomerative.py#L762-L780

Labels: Documentation
Document the class DistanceMetric

Right now we don't have any documentation for the public class `DistanceMetric`.
It would be nice to have a description.

xref: https://scikit-learn.org/dev/modules/generated/sklearn.metrics.DistanceMetric.html#sklearn.metrics.DistanceMetric

COMMENT:
Oh it means their is need to make changes here:

https://github.com/scikit-learn/scikit-learn/blob/d7605c4e34b752ef9f200238eeb8de498b559f55/sklearn/metrics/_dist_metrics.pyx.tp#L66-L82

Labels: Documentation
Document the class DistanceMetric

Right now we don't have any documentation for the public class `DistanceMetric`.
It would be nice to have a description.

xref: https://scikit-learn.org/dev/modules/generated/sklearn.metrics.DistanceMetric.html#sklearn.metrics.DistanceMetric

COMMENT:
adding doc reference of `DistanceMetric` before moving from `Neighbours `to `Metric`. Might be useful
[https://scikit-learn.org/0.24/modules/generated/sklearn.neighbors.DistanceMetric.html](https://scikit-learn.org/0.24/modules/generated/sklearn.neighbors.DistanceMetric.html)

Labels: Documentation
Document the class DistanceMetric

Right now we don't have any documentation for the public class `DistanceMetric`.
It would be nice to have a description.

xref: https://scikit-learn.org/dev/modules/generated/sklearn.metrics.DistanceMetric.html#sklearn.metrics.DistanceMetric

COMMENT:
@Shreesha3112 you're giving a link from a very old release which we're no longer supporting.

Labels: Documentation
Document the class DistanceMetric

Right now we don't have any documentation for the public class `DistanceMetric`.
It would be nice to have a description.

xref: https://scikit-learn.org/dev/modules/generated/sklearn.metrics.DistanceMetric.html#sklearn.metrics.DistanceMetric

COMMENT:
> @Shreesha3112 you're giving a link from a very old release which we're no longer supporting.

I just thought that metric mapping stuff may be useful as reference even though it is old.

Labels: Documentation
making mutual_info_regression more verbose

### Describe the workflow you want to enable

Hi, 

Thanks for coding up `mutual_info_regression`, I use it for novel space science research and it's super helpful for me.

I'm currently using it to analyse quite large amounts of data, and understandably this can be quite slow. I'm wondering if it would be possible to implement an optional flag in `mutual_info_regression` to make the code verbose while it's running. For example, printing out something to the terminal when it is 10%, 20%, 30% (etc) done.

Just an idea - hopefully I have raised this in the correct way!

Thanks,
Alexandra Fogg

### Describe your proposed solution

Implementation of an optional flag in `mutual_info_regression` to make the function verbose, where it prints out statements at certain stages of the process, e.g. 10% done or saying that a certain stage is completed, etc.

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

_No response_

### Additional context

_No response_

COMMENT:
I don't know if there is precedent for having progress bars in scikit-learn. I don't think so? Unless there is precedent I think we shouldn't add it. I think if people want a progress bar or some other kind of "spinner" it should be implemented "on top of" scikit-learn. The reason for this is that there are plenty of things in scikit-learn that could/should display a progress bar and then you have progress bars inside of progress bars, etc. And you need to somehow choose when to show a progress bar, etc.

All these things are in some way easier to do in a library that uses scikit-learn (showing just one progress bar, knowing which bits are slow, etc).

Labels: New Feature