[MRG] Support for infinite values in GBDTs#14406
[MRG] Support for infinite values in GBDTs#14406ogrisel merged 3 commits intoscikit-learn:masterfrom
Conversation
| # This is not strictly True, but it's needed since | ||
| # force_all_finite=False means accept both nans and infinite values. | ||
| # Without the tag, common checks would fail. | ||
| # This comment must be removed once we merge PR 13911 |
There was a problem hiding this comment.
Maybe add a "TODO", we sometimes go through them and it'll be easier to find it then. But if you're gonna fix it yourself, then no big deal.
|
ping when tests pass? |
|
ping @adrinjalali They pass ^^ it's a docker issue |
ogrisel
left a comment
There was a problem hiding this comment.
LGTM. Just a quick comment to make the atol in a test more easy to understand but not big deal. Feel free to merge without addressing it if you don't like my suggestion :)
|
|
||
| gbdt = HistGradientBoostingRegressor(min_samples_leaf=1) | ||
| gbdt.fit(X, y) | ||
| np.testing.assert_allclose(gbdt.predict(X), y, atol=1e-4) |
There was a problem hiding this comment.
Why such a high value for atol? Maybe max_iter it too small for the default value of the learning rate? Maybe you could set the learning rate to 1.0 and a single split in a single tree (max_iter=1, max_leaf_nodes=2)would be enough to perfectly fit the data?
|
I launched a rebuild of azure and circle as the failures did not look related to this PR. |
|
The tests pass. Let's merge, we can always improve the test later :) |
ping @ogrisel @adrinjalali
I think we need this merged before the missing values support :)