[MRG] Partial dependence plots -- continued #12599

NicolasHug · 2018-11-15T20:31:01Z

Reference Issues/PRs

This is the continuation of #5653. Lots of tests and checks have been added, with a few fixes. More detailed changes from #5653 can be found here #5653 (comment).

Closes #5653
Closes #4405

What does this implement/fix? Explain your changes.

This PR implements support for partial dependence plots with any regressor or classifier that has predict_proba.

Any other comments?

Partial dependence plots were only supported for gradient boosting trees and the functions were in ensemble.partial_dependence. All backward compatibility is preserved, but those functions are deprecated in favor of the sklearn.partial_dependence module.

The grid param has been removed in the new implementation.
The ax param was replaced by fig. The claim that the new plots were plotted on the ax argument was wrong.

Some possible improvements

In separate PRs:

Add support for 3D plots instead of just a contour
Add support for https://arxiv.org/pdf/1309.6392.pdf. Basically, instead of averaging the responses, we actually plot all of them. That would only be relevant for 1-way PDP.
Add support for plotting the same PDP of different estimators on the same matplotlib ax object.

…earn#7846

glemaitre

I made the change to the CI to trigger the changes.

jnothman

Can you give me a sense of what else has changed? It's hard to get from comparing commits since files have moved etc.

doc/whats_new/v0.21.rst

sklearn/inspection/partial_dependence.py

NicolasHug · 2019-04-24T01:31:54Z

Can you give me a sense of what else has changed? It's hard to get from comparing commits since files have moved etc.

Apart from moving files to inspection, not much has changed since you last saw it.

jnothman · 2019-04-24T07:41:34Z

I'm happy to merge after those three comments are addressed.

… partial_dep

jnothman · 2019-04-24T12:52:26Z

Thanks and congrats, @NicolasHug and @trevorstephens!!

NicolasHug · 2019-04-24T13:10:32Z

Thanks a lot for the reviews @glemaitre @jnothman

trevorstephens · 2019-04-24T16:02:01Z

Wow, congrats @NicolasHug on getting this done, bravo!!

amueller · 2019-04-24T16:07:40Z

Yay!!

This reverts commit e6c7e94.

ralphhaygood · 2020-06-19T22:05:22Z

The grid param has been removed in the new implementation

Which breaks my package sklearn-gbmi (which computes Friedman-Popescu H statistics to identify interactions between features in gradient boosted regression trees).

Looking at the source code, I don't understand why you did this. It seems to me the function could still accommodate the specified-grid operating mode.

Anyhow, is there some simple work-around I'm not seeing? I'd appreciate your advice.

ralphhaygood · 2020-06-19T22:06:58Z

GitHub doesn't seem to be making it obvious, so I'll clarify that the preceding comment is directed to @NicolasHug.

NicolasHug · 2020-06-19T22:24:29Z

Honestly I don't exactly remember.

I assume it's because with the additional support of the brute method, the interactions between method, X and grid were becoming too intricate to be reasonably intuitive.

Unless something went wrong, you should have been warned long ago about the deprecation and ultimate removal of these previous functions.

NicolasHug · 2020-06-19T22:25:32Z

Anyhow, is there some simple work-around I'm not seeing? I'd appreciate your advice.

You can still pass X and the grid will be automatically computed from there. Though I'm not sure that solves your issue

ralphhaygood · 2020-06-19T23:50:46Z

Unless something went wrong, you should have been warned long ago about the deprecation and ultimate removal of these previous functions.

I wasn't. I was aware of your efforts to support a broader class of models, which I applaud. I wasn't aware of the removal of the specified-grid operating mode until users of my package informed me the package was broken. (I haven't been using it myself recently.)

Though I'm not sure that solves your issue

It doesn't.

I could work around the problem by writing my own version of the partial_dependence function, supporting exactly my use case. However, it would depend on functions that aren't parts of the public API and that therefore could be changed at any time. It would be safer to create my own complete implementation. I don't have time to deal with either option right now, but I'll decide what to do and do it early next month.

jnothman · 2020-06-20T23:56:52Z

I think there may have been no explicit warning about grid because backwards compatibility issues disappeared when we moved this to a different subpackage. It might be appropriate to solve this usage, but it would be best reported as a separate issue rather than a comment on a merged pull request @ralphhaygood.

trevorstephens and others added 30 commits May 25, 2018 18:51

general partial dependence plots

57f9a6f

add init

9a09888

implement exact and estimated methods

e714e16

support for Pipeline and GridSearchCV type estimators

19ed28e

add multioutput support

6fafc5e

rebase and catch up to scikit-learn#6762, scikit-learn#7673, scikit-l…

152a190

…earn#7846

catch up on scikit-learn#9434

2cdc5ea

initial update of plot_partial_dependence

ba1f8da

deprecate ensemble.partial_dependence

1b1d8f0

refactor estimated and exact functions to _predict

9095305

make "auto" the default rather than None for method

3fc1727

some more refactoring

259ec99

avoid namespace collision

cbc20af

fix output shapes of all estimators

63da115

add tests to ensure all estimators output same shape

8f7d2b0

quick fixes

6fc3a49

fix docstring, test fails

b1f8bfc

refactor tests for easier debugging

dc93b69

speed up tests, add two-way plot test

cd8f8de

move input validation on X

4eb1a80

fix output shape for multi-label classification

21544ce

update plot helper to support multi-output

610b5c5

update plot helper to pass-through output

dcbd0c6

Merge branch 'master' into partial_dep

08ef804

removed estimated method, small refactoring

3f5c7f7

factorized some test

45e648d

some more refactoring

ba4868f

test for _grid_from_X

414387b

few changes and comments

f635693

some test + removed multioutput logic for now

58bfbad

glemaitre approved these changes Apr 23, 2019

View reviewed changes

jnothman requested changes Apr 24, 2019

View reviewed changes

doc/whats_new/v0.21.rst Outdated Show resolved Hide resolved

sklearn/inspection/partial_dependence.py Outdated Show resolved Hide resolved

thomasjpfan reviewed Apr 24, 2019

View reviewed changes

sklearn/inspection/partial_dependence.py Outdated Show resolved Hide resolved

NicolasHug added 4 commits April 24, 2019 07:30

Addressed comments

d22d7b2

Merge branch 'partial_dep' of github.com:NicolasHug/scikit-learn into…

80f53cb

… partial_dep

Merge remote-tracking branch 'upstream/master' into partial_dep

5677050

forgot if_mpl decorator

27c261e

jnothman approved these changes Apr 24, 2019

View reviewed changes

jnothman merged commit 6b7dc9b into scikit-learn:master Apr 24, 2019

NicolasHug mentioned this pull request Apr 24, 2019

Document matplotlib dependency/ install matplotlib in CI #13335

Closed

jeremiedbb pushed a commit to jeremiedbb/scikit-learn that referenced this pull request Apr 25, 2019

FEA Partial dependence plots (scikit-learn#12599)

5135252

NicolasHug mentioned this pull request Apr 26, 2019

[MRG] removed close_figure helper #13730

Merged

xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019

FEA Partial dependence plots (scikit-learn#12599)

e6c7e94

xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019

Revert "FEA Partial dependence plots (scikit-learn#12599)"

f73d2b2

This reverts commit e6c7e94.

xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019

Revert "FEA Partial dependence plots (scikit-learn#12599)"

8e4cf27

This reverts commit e6c7e94.

zd-project mentioned this pull request Jun 19, 2019

Add ICE to partial dependence plot #14126

Closed

koenvandevelde pushed a commit to koenvandevelde/scikit-learn that referenced this pull request Jul 12, 2019

FEA Partial dependence plots (scikit-learn#12599)

f3ac42b

thomasjpfan mentioned this pull request Jul 14, 2019

[MRG] Plotting API starting with ROC curve #14357

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MRG] Partial dependence plots -- continued #12599

[MRG] Partial dependence plots -- continued #12599

NicolasHug commented Nov 15, 2018 •

edited

Loading

glemaitre left a comment

jnothman left a comment

NicolasHug commented Apr 24, 2019

jnothman commented Apr 24, 2019

jnothman commented Apr 24, 2019

NicolasHug commented Apr 24, 2019

trevorstephens commented Apr 24, 2019

amueller commented Apr 24, 2019

ralphhaygood commented Jun 19, 2020

ralphhaygood commented Jun 19, 2020

NicolasHug commented Jun 19, 2020

NicolasHug commented Jun 19, 2020

ralphhaygood commented Jun 19, 2020

jnothman commented Jun 20, 2020 via email

[MRG] Partial dependence plots -- continued #12599

[MRG] Partial dependence plots -- continued #12599

Conversation

NicolasHug commented Nov 15, 2018 • edited Loading

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Some possible improvements

glemaitre left a comment

Choose a reason for hiding this comment

jnothman left a comment

Choose a reason for hiding this comment

NicolasHug commented Apr 24, 2019

jnothman commented Apr 24, 2019

jnothman commented Apr 24, 2019

NicolasHug commented Apr 24, 2019

trevorstephens commented Apr 24, 2019

amueller commented Apr 24, 2019

ralphhaygood commented Jun 19, 2020

ralphhaygood commented Jun 19, 2020

NicolasHug commented Jun 19, 2020

NicolasHug commented Jun 19, 2020

ralphhaygood commented Jun 19, 2020

jnothman commented Jun 20, 2020 via email

NicolasHug commented Nov 15, 2018 •

edited

Loading