BUG Fix zero division error in GBDTs #14024
Conversation
| @@ -2352,8 +2352,8 @@ def check_decision_proba_consistency(name, estimator_orig): | |||
| hasattr(estimator, "predict_proba")): | |||
|
|
|||
| estimator.fit(X, y) | |||
| a = estimator.predict_proba(X_test)[:, 1] | |||
| b = estimator.decision_function(X_test) | |||
| a = estimator.predict_proba(X_test)[:, 1].round(decimals=10) | |||
There was a problem hiding this comment.
That's not great :-/ Can we avoid that? Basically you're saying after the "fix" the estimator is not consistent any more because of floating point issues?
There was a problem hiding this comment.
because scipy's expit isn't precise enough. But I feel like 1e-10 is a decent precision. I had issues with this test before and IMO it's too strict.
There was a problem hiding this comment.
Should we be rounding as a non-copying operation? Is this copying?
There was a problem hiding this comment.
It's copying.
I can't tell from the docs (nor the code) whether it copies if we pass out=.
I don't think that's important though?
There was a problem hiding this comment.
Using out= will not copy if it's into the same array.
I forgot this was in estimator checks. Yes, not important.
| @@ -2352,8 +2352,8 @@ def check_decision_proba_consistency(name, estimator_orig): | |||
| hasattr(estimator, "predict_proba")): | |||
|
|
|||
| estimator.fit(X, y) | |||
| a = estimator.predict_proba(X_test)[:, 1] | |||
| b = estimator.decision_function(X_test) | |||
| a = estimator.predict_proba(X_test)[:, 1].round(decimals=10) | |||
There was a problem hiding this comment.
Should we be rounding as a non-copying operation? Is this copying?
sklearn/ensemble/_hist_gradient_boosting/tests/test_gradient_boosting.py
Show resolved
Hide resolved
|
Comment was addressed |
Fixes #14018
Probably also #14014
Non-regression test fails on master.