Skip to content

[MRG] DOC add permutation importance to GradientBoostingRegressor#16742

Merged
adrinjalali merged 5 commits intoscikit-learn:masterfrom
nilichen:gbr_permutation_imp
Mar 24, 2020
Merged

[MRG] DOC add permutation importance to GradientBoostingRegressor#16742
adrinjalali merged 5 commits intoscikit-learn:masterfrom
nilichen:gbr_permutation_imp

Conversation

@nilichen
Copy link
Copy Markdown
Contributor

@nilichen nilichen commented Mar 21, 2020

Reference Issues/PRs

Towards #14528.

What does this implement/fix? Explain your changes.

The latest doc: https://97558-843222-gh.circle-artifacts.com/0/doc/auto_examples/ensemble/plot_gradient_boosting_regression.html

The previous way of having one plot but separate code blocks didn't really render as expected (https://97540-843222-gh.circle-artifacts.com/0/doc/auto_examples/ensemble/plot_gradient_boosting_regression.html)

image

@nilichen nilichen force-pushed the gbr_permutation_imp branch 4 times, most recently from 200cbd4 to 5d89ab6 Compare March 22, 2020 00:31
@nilichen nilichen force-pushed the gbr_permutation_imp branch from 5d89ab6 to 7b02f05 Compare March 22, 2020 00:56
@nilichen nilichen changed the title DOC add permutation importance to GradientBoostingRegressor [MRG] DOC add permutation importance to GradientBoostingRegressor Mar 22, 2020
Copy link
Copy Markdown
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.

LGTM, can you just please add a small note, either in the top level comment or an inline comment just before or right after the plot to give a short analysis of the results: in particular the two methods agree to identify the same top 2 features as strongly predictive features but not in the same order.

The third most predictive feature, "bp", is also the same for the 2 methods. The remaining features are less predictive and the error bars of the permutation plot show that they overlap with 0.

@nilichen nilichen force-pushed the gbr_permutation_imp branch 2 times, most recently from 266cf89 to 117ad5d Compare March 23, 2020 07:36
@nilichen nilichen force-pushed the gbr_permutation_imp branch from 117ad5d to b478aee Compare March 23, 2020 07:47
@nilichen
Copy link
Copy Markdown
Contributor Author

@ogrisel Thanks for the suggestion! Note added.

Copy link
Copy Markdown
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

Thanks @nilichen. Other than the minor point LGTM.

Please also refrain from force pushing an amended history instead of pushing new commits. We'll squash and merge and all your commits will become one at the end anyway :)

Co-Authored-By: Adrin Jalali <adrin.jalali@gmail.com>
@nilichen
Copy link
Copy Markdown
Contributor Author

@adrinjalali Got it.

@adrinjalali adrinjalali merged commit 104b736 into scikit-learn:master Mar 24, 2020
gio8tisu pushed a commit to gio8tisu/scikit-learn that referenced this pull request May 15, 2020
…earn#16742)

* add permutation importance to gbr

* format document

* separate deviance from feature importance during plotting

* add some analysis

* Update examples/ensemble/plot_gradient_boosting_regression.py

Co-Authored-By: Adrin Jalali <adrin.jalali@gmail.com>

Co-authored-by: Katrina Ni <kani@tableau.com>
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants