-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
FIX Fixes HistGradientBoosting bug fail when early stopping + no validation + warm starting #16662
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
… within each split (scikit-learn#16085)
* removed warn_on_dtype * removed parameters to check_is_fitted * all_estimators parameters * deprecated n_components attribute in AgglomerativeClustering * change default of base.score for multioutput * removed lots of useless decorators? * changed default of copy in quantil_transform * removed six.py * nmf default value of init param * raise error instead of warning in LinearDiscriminantAnalysis * removed label param in hamming_loss * updated method parameter of power_transform * pep8 * changed default value of min_impurity_split * removed assert_false and assert_true * added and fixed versionchanged directives * reset min_impurity_split default to None * fixed LDA issue * fixed some test * more docstrings updates * set min_impurity_decrease for test to pass * upate docstring example * fixed doctest * removed multiouput.score since it's now consistent with the default * deprecate least_angle parameter combination * remove support for l1 or l2 loss in svm * removed linear_assignment.py * add test
…ikit-learn#14300) Co-authored-by: Christian Lorentzen <lorentzen.ch@googlemail.com> Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org> Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>
Co-authored-by: JohanWork
…n#16585) Co-authored-by: Thomas J Fan <thomasjpfan@gmail.com> Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
@@ -645,3 +645,16 @@ def test_max_depth_max_leaf_nodes(): | |||
tree = est._predictors[0][0] | |||
assert tree.get_max_depth() == 2 | |||
assert tree.get_n_leaf_nodes() == 3 # would be 4 prior to bug fix | |||
|
|||
|
|||
def test_early_stopping_on_test_set_with_warm_start(): |
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 doesn't fail on master, nor does the original snippet from the issue
else: | ||
raw_predictions_val = None |
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.
I think we should put this higher up e.g. line 306.
github is ui for changing the branch to merge into is interesting... Closing for now, this is a bug in 0.22.X and not in master. |
Reference Issues/PRs
Fixes #16661
What does this implement/fix? Explain your changes.
Sets variable when there is no validation set.
Any other comments?
Should we backport this fix? With #14516 it would require a version check to pass the correct parameters in the test.