Skip to content
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

DOC Point GradientBoosting towards HistGradientBoosting #23340

Merged
merged 10 commits into from May 24, 2022

Conversation

ArturoAmorQ
Copy link
Member

Reference Issues/PRs

Addresses #23231.

What does this implement/fix? Explain your changes.

People may use GradientBoosting* in cases where HistGradientBoosting* would perform better.
This PR addresses the problem by adding references towards the Hist versions in the docstrings and examples.

Any other comments?

This is a quick fix as suggested by this comment.

Reworking the examples (as mentioned in this comment) or adding an additional example (as mentioned here) is left for another PR.

Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. I think we should avoid the "GB" acronym and rather be explicit at the cost of a bit of redundancy.

examples/applications/plot_model_complexity_influence.py Outdated Show resolved Hide resolved
sklearn/ensemble/_gb.py Outdated Show resolved Hide resolved
Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more suggestions but LGTM.

sklearn/ensemble/_gb.py Outdated Show resolved Hide resolved
sklearn/ensemble/_gb.py Outdated Show resolved Hide resolved
ArturoAmorQ and others added 2 commits May 24, 2022 10:47
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
Copy link
Member

@jjerphan jjerphan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for cross-referencing the HGBT interfaces, @ArturoAmorQ.

I think the terminology can be made a bit more precise, for instance by replacing "estimator" by "algorithm". Here are a few comments in this regard.

examples/applications/plot_model_complexity_influence.py Outdated Show resolved Hide resolved
examples/ensemble/plot_feature_transformation.py Outdated Show resolved Hide resolved
examples/ensemble/plot_gradient_boosting_quantile.py Outdated Show resolved Hide resolved
sklearn/ensemble/_gb.py Outdated Show resolved Hide resolved
sklearn/ensemble/_gb.py Outdated Show resolved Hide resolved
ArturoAmorQ and others added 3 commits May 24, 2022 11:51
Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>
Copy link
Member

@jjerphan jjerphan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you, @ArturoAmorQ!

@jjerphan jjerphan merged commit 89c8352 into scikit-learn:main May 24, 2022
@ArturoAmorQ ArturoAmorQ deleted the GBDT_to_HGBDT branch June 9, 2022 13:30
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Aug 4, 2022
…#23340)

* Point GradientBoosting towards HistGradientBoosting on docstrings

* Point GradientBoosting towards HistGradientBoosting on examples

* Avoid the GB acronym

* Iter

* Apply suggestion from lesteve

* Apply suggestions from code review

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

* Format

* Apply suggestions from code review

Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>

* Format

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>
glemaitre pushed a commit that referenced this pull request Aug 5, 2022
* Point GradientBoosting towards HistGradientBoosting on docstrings

* Point GradientBoosting towards HistGradientBoosting on examples

* Avoid the GB acronym

* Iter

* Apply suggestion from lesteve

* Apply suggestions from code review

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

* Format

* Apply suggestions from code review

Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>

* Format

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>
mathijs02 pushed a commit to mathijs02/scikit-learn that referenced this pull request Dec 27, 2022
…#23340)

* Point GradientBoosting towards HistGradientBoosting on docstrings

* Point GradientBoosting towards HistGradientBoosting on examples

* Avoid the GB acronym

* Iter

* Apply suggestion from lesteve

* Apply suggestions from code review

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

* Format

* Apply suggestions from code review

Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>

* Format

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants