8000 RFC deprecate the SAMME.R algorithm in AdaBoostClassifier · Issue #26784 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

RFC deprecate the SAMME.R algorithm in AdaBoostClassifier #26784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
glemaitre opened this issue Jul 6, 2023 · 11 comments · Fixed by #26830
Closed

RFC deprecate the SAMME.R algorithm in AdaBoostClassifier #26784

glemaitre opened this issue Jul 6, 2023 · 11 comments · Fixed by #26830

Comments

@glemaitre
Copy link
Member
glemaitre commented Jul 6, 2023

While working on the following example (#26486), I found a couple of issues regarding the SAMME.R algorithm, which is the default algorithm in AdaBoostClassifier:

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

@github-actions github-actions bot added the Needs Triage Issue requires triage label Jul 6, 2023
@adrinjalali
Copy link
Member

I'd be +1 for that.

(Also a good issue for @StefanieSenger if we decide to do it)

@thomasjpfan
Copy link
Member

However, this paper is a preprint. In the final version of the paper, the SAMME.R algorithm is presented.

@glemaitre Did you mean that SAMME.R is not present in the published paper?

@glemaitre
Copy link
Member Author

Yes, it is not published. I corrected my sentence.

@jpangas
Copy link
Contributor
jpangas commented Jul 12, 2023

Is @StefanieSenger working on this already? If not, I would be willing to take it up.:)

@StefanieSenger
Copy link
Contributor

Hey @jpangas, I'm working on it already. :)

@thomasjpfan
Copy link
Member

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

@thomasjpfan thomasjpfan added API RFC module:ensemble and removed Needs Triage Issue requires triage labels Jul 12, 2023
@adrinjalali
Copy link
Member

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.

@glemaitre
Copy link
Member Author

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.

@StefanieSenger
Copy link
Contributor

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?

@StefanieSenger
Copy link
Contributor

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

@glemaitre
Copy link
Member Author

The first version will make it easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
0