-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
[MRG+2] Early stopping for Gradient Boosting Classifier/Regressor #7071
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
Conversation
mean = np.mean(scores) | ||
params['n_estimators'] = int(np.mean(n_est_list)) | ||
score_tuple = _CVScoreTuple(params, mean, scores) | ||
grid_scores.append(_CVScoreTuple(params, mean, scores)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe. We should not use _CVScoreTuple
, we should try to get rid of that one. The other EstimatorCV
models implement scores_
, right? I feel like they should all have a consistent interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A consistent interface that is like GridSearchCV
's results_
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'd like to see a consistent interface like GridSearchCV
's results, and always had that in mind. I don't suppose we'll see it in 0.18. Sorry I wasn't fastidious enough to post this as a new issue months ago.
Also any advice on what kind of tests I should add here? |
And finally would like to know
|
ba42a94
to
5d742df
Compare
tests: that this does the same as naive grid-search and that it stops early when it should. |
doc/modules/classes.rst
Outdated
@@ -383,6 +383,7 @@ Samples generator | |||
ensemble.ExtraTreesRegressor | |||
ensemble.GradientBoostingClassifier | |||
ensemble.GradientBoostingRegressor | |||
ensemble.GradientBoostingClassifierCV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RegressorCV also
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added below
|
for train, validation in cv.split(X) | ||
for params in param_iter) | ||
|
||
n_splits = int(len(out)/len(param_iter)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pep8, also //
@amueller could you enter captcha before reviewing? :p |
I remember distinctly that you asked for a review yesterday ;) |
Thanks a tonne for looking into the PR. I'll address them and update you! |
gb.n_estimators = i | ||
gb.fit(X_train, y_train) | ||
|
||
scores = np.roll(scores, shift=-1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems an odd way of doing this. Why not append to a list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a way to retain the last 10 scores. Hence a shift register kinda setup.
doc/whats_new.rst
Outdated
via ``n_iter_no_change``, ``validation_fraction`` and `tol`. :issue:`7071` | ||
by `Raghav RV`_ | ||
|
||
- Added :class:`multioutput.ClassifierChain` for multi-label |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
merge error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No this is in master. I don't why it shows in the diff.
doc/whats_new.rst
Outdated
@@ -81,6 +81,13 @@ New features | |||
|
|||
Classifiers and regressors | |||
|
|||
- :class:`ensemble.GradientBoostingClassifier` and | |||
:class:`ensemble.GradientBoostingRegressor` now support early stopping | |||
via ``n_iter_no_change``, ``validation_fraction`` and `tol`. :issue:`7071` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did tol only get single backticks? ;)
given that there are a couple of PRs about related interfaces open, I think
we have extra reason to not release this in 0.19
…On 28 Jul 2017 12:44 am, "(Venkat) Raghav, Rajagopalan" < ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In doc/whats_new.rst
<#7071 (comment)>
:
> @@ -81,6 +81,13 @@ New features
Classifiers and regressors
+ - :class:`ensemble.GradientBoostingClassifier` and
+ :class:`ensemble.GradientBoostingRegressor` now support early stopping
+ via ``n_iter_no_change``, ``validation_fraction`` and `tol`. :issue:`7071`
+ by `Raghav RV`_
+
+ - Added :class:`multioutput.ClassifierChain` for multi-label
No this is in master. I don't why it shows in the diff.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7071 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEz69PvXrc0Z96yS97dam-7KmKrQEZaks5sSKJCgaJpZM4JTmPM>
.
|
@jnothman Have moved the whatsnew entry into a new section - 0.20 |
This example illustrates how the early stopping can used in the | ||
:class:`sklearn.ensemble.GradientBoostingClassifier` model to achieve | ||
almost the same accuracy as compared to a model built without early stopping | ||
using many fewer estimators. This can save memory and prediction time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can significantly reduce training time, memory usage and prediction latency.
early stopping. Must be between 0 and 1. | ||
Only used if ``n_iter_no_change`` is set to an integer. | ||
|
||
.. versionadded:: 0.19 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be changed to 0.20.
early stopping. Must be between 0 and 1. | ||
Only used if early_stopping is True | ||
|
||
.. versionadded:: 0.19 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment for this class docstring: 0.20.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM besides small comments.
def test_gradient_boosting_validation_fraction(): | ||
X, y = make_classification(n_samples=1000, random_state=0) | ||
|
||
gbc = GradientBoostingClassifier(n_estimators=100, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment by @jnothman has not been addressed.
def test_gradient_boosting_validation_fraction(): | ||
X, y = make_classification(n_samples=1000, random_state=0) | ||
|
||
gbc = GradientBoostingClassifier(n_estimators=100, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still use nosetest in travis and appveyor at this time though.
Merged. Thanks for this nice contribution @raghavrv! |
Finishing up @vighneshbirodkar's #5689 (Also refer #1036)
Enables early stopping to gradient boosted models via new parameters
n_iter_no_change
,validation_fraction
,tol
.(This takes inspiration from our
MLPClassifier
)This has been rewritten after IRL discussions with @agramfort and @ogrisel.
@amueller @agramfort @MechCoder @vighneshbirodkar @ogrisel @glouppe @pprett