Skip to content

MAINT accelerate plot_partial_dependence.py #21768

Merged
thomasjpfan merged 3 commits intoscikit-learn:mainfrom
siavrez:partial_dependence
Dec 15, 2021
Merged

MAINT accelerate plot_partial_dependence.py #21768
thomasjpfan merged 3 commits intoscikit-learn:mainfrom
siavrez:partial_dependence

Conversation

@siavrez
Copy link
Copy Markdown
Contributor

@siavrez siavrez commented Nov 23, 2021

Changed MPLRegressor layers from (50, 50) to (30, 15) added random_state=0
R2 is 0.82 from previous 0.81
Added max_depth=7 and random_state=0 to HistGradientBoostingRegressor
R2 is the same 0.85
Prevously preceptrons convergence was the main time consuming element and for different runs it was between 25 seconds to 48 seconds
with new params it's 18.3.
Also added auto_add_to_figure=False to matplotlib Axe3D to selent depecation warning.

Reference Issues/PRs

#21598

What does this implement/fix? Explain your changes.

Any other comments?

@adrinjalali adrinjalali mentioned this pull request Nov 24, 2021
41 tasks
print("Training HistGradientBoostingRegressor...")
tic = time()
est = HistGradientBoostingRegressor()
est = HistGradientBoostingRegressor(max_depth=7, random_state=0)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would let the gradient boosting as-is. There is little gain here.

Copy link
Copy Markdown
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

Could you fix the n_jobs=2 and the grid_resolution=10 to check if it would be OK. I assume that such a resolution would be enough to discuss the tendency on the plots.

@siavrez
Copy link
Copy Markdown
Contributor Author

siavrez commented Nov 25, 2021

HistGradientBoostingRegressor() runtime was 3.3 seconds.
HistGradientBoostingRegressor(max_depth=7) 1.9 seconds.
After updating main branch HistGradientBoostingRegressor() runtime is 1-79 seconds!
I've tested it with 200 runs. it's not related to params or seed.

@siavrez
Copy link
Copy Markdown
Contributor Author

siavrez commented Nov 25, 2021

Could you fix the n_jobs=2 and the grid_resolution=10 to check if it would be OK. I assume that such a resolution would be enough to discuss the tendency on the plots.

There is slight improvement on plots rendering runtime after changing these params.

@siavrez
Copy link
Copy Markdown
Contributor Author

siavrez commented Nov 25, 2021

MatplotlibDeprecationWarning for Axes3D can be removed by adding auto_add_to_figure=False to Axes3D but a test on ckecking signatures would fail.

@siavrez siavrez requested a review from glemaitre November 28, 2021 09:43
Copy link
Copy Markdown
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

LGTM. I think that we can let the warning for the moment. It could be done in another PR.

Copy link
Copy Markdown
Member

@thomasjpfan thomasjpfan 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 the PR @siavrez ! LGTM

@thomasjpfan thomasjpfan changed the title accelerate plot_partial_dependence.py MAINT accelerate plot_partial_dependence.py Dec 15, 2021
@thomasjpfan thomasjpfan merged commit 845b1fa into scikit-learn:main Dec 15, 2021
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Dec 24, 2021
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