Skip to content

Adaboost Learning Rate Documentation #19521

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
mnowicki6 opened this issue Feb 21, 2021 · 6 comments · Fixed by #19919
Closed

Adaboost Learning Rate Documentation #19521

mnowicki6 opened this issue Feb 21, 2021 · 6 comments · Fixed by #19919

Comments

@mnowicki6
Copy link

Describe the issue linked to the documentation

The documentation on this page https://scikit-learn.org/stable/modules/generated/sklearn.ensemble.AdaBoostClassifier.html

Describes the learning rate as "Learning rate shrinks the contribution of each classifier by learning_rate. There is a trade-off between learning_rate and n_estimators."

The wording of this description suggests that a higher learning rate = more shrinking of the contribution of each classifier by learning_rate.

Suggest a potential alternative/fix

The description would improve by replacing the word "shrinks," or by providing an example.

E.g.,
"Learning rate adjusts the contribution of each classifier..."
or
"Learning rate shrinks the contribution . . . where a higher learning rate increases the contribution of each each classifier"

@thomasjpfan
Copy link
Member

I agree that the docstring can be improved. I like the second option with the first part removed:

A higher learning rate increases the contribution of each additional classifier.

Because it gets straight to the point. What do you think?

@mnowicki6
Copy link
Author

mnowicki6 commented Feb 22, 2021

Actually, I might remove "additional" now that I think about it.

"A higher learning rate increases the contribution of each classifier"

Otherwise agreed!

@thomasjpfan
Copy link
Member

If you want, you can open a pull request with your suggestion.

@ravisripadak
Copy link

Can I fix this?

@cmarmo
Copy link
Contributor

cmarmo commented Mar 18, 2021

Hi @ravisripadak feel free to open a pull request if you are interested in working on this. You can also comment "Take" in the issue to have it automatically assigned to yourself.

@mnowicki6
Copy link
Author

Take

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.

4 participants