Skip to content

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

Closed
wants to merge 448 commits into from

Conversation

thomasjpfan
Copy link
Member

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.

cmarmo and others added 30 commits January 9, 2020 14:15
* 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
maikia and others added 25 commits March 3, 2020 15:33
…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>
…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():
Copy link
Member

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

Comment on lines +399 to +400
else:
raw_predictions_val = None
Copy link
Member

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.

@thomasjpfan thomasjpfan changed the base branch from master to 0.22.X March 10, 2020 16:41
@thomasjpfan
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment