MAINT Clean up deprecations for 1.5: in log_loss #28851
Conversation
| # binary case: check correct boundary values for eps = 0 | ||
| with pytest.warns(FutureWarning): | ||
| assert log_loss([0, 1], [0, 1], eps=0) == 0 | ||
| with pytest.warns(FutureWarning): | ||
| assert log_loss([0, 1], [0, 0], eps=0) == np.inf | ||
| with pytest.warns(FutureWarning): | ||
| assert log_loss([0, 1], [1, 1], eps=0) == np.inf | ||
|
|
||
| # multiclass case: check correct boundary values for eps = 0 | ||
| with pytest.warns(FutureWarning): | ||
| assert log_loss([0, 1, 2], [[1, 0, 0], [0, 1, 0], [0, 0, 1]], eps=0) == 0 | ||
| with pytest.warns(FutureWarning): | ||
| assert ( | ||
| log_loss([0, 1, 2], [[0, 0.5, 0.5], [0, 1, 0], [0, 0, 1]], eps=0) == np.inf | ||
| ) |
There was a problem hiding this comment.
@lorentzenchr I'm a bit confused. Does removing eps (deprecated in #25299) means that now eps is always 0 or eps is always computed based on the dtype ? The previous "auto" seems to indicate the latter but in that case testing edge cases is no longer possible.
There was a problem hiding this comment.
See #24515 (comment).
My opinion is that eps=0 is the correct behavior (who are we to judge and MODIFY uncalibrated predicted probabilities!). The consensus was more in the direction of dtype dependent.
There was a problem hiding this comment.
At least, the clipping should not happen here to me. If y_true=0 and y_pred=0, the result should be exactly 0. xlogy(0, 0) = 0 (no warning).
The question is do we want to return inf when y_true != 0 and y_pred = 0, or a finite value. If the former, we should clip with eps=0, else we should clip the result of xlogy as suggested in #24515 (comment)
I would go with returning inf, but I don't know if we rely on it being finite (maybe if *SearchCV and co), and the warning message said that eps will be non-zero in 1.5, so maybe we should better keep it as is.
There was a problem hiding this comment.
I have the same opinion as you.
There was a problem hiding this comment.
Indeed, I find the following weird:
>>> log_loss([0, 1, 2], [[1, 0, 0], [0, 1, 0], [0, 0, 1]])
2.2204460492503136e-16Shall to conditionally clip to eps only when one_hot_encode(y_true) > 0 and clip to 0 otherwise?
There was a problem hiding this comment.
Actually whatever the decision on this, there should be a test to cover the case where log_loss reaches its minimum (perfect predictions), both in binary and multiclass settings.
There was a problem hiding this comment.
I'd rather leave this discussion for a separate issue/PR to (try to) keep the focus of this PR on the deprecations clean-up.
I added a test for perfect predictions that only checks that the result is close to 0 for now.
| # binary case: check correct boundary values for eps = 0 | ||
| with pytest.warns(FutureWarning): | ||
| assert log_loss([0, 1], [0, 1], eps=0) == 0 | ||
| with pytest.warns(FutureWarning): | ||
| assert log_loss([0, 1], [0, 0], eps=0) == np.inf | ||
| with pytest.warns(FutureWarning): | ||
| assert log_loss([0, 1], [1, 1], eps=0) == np.inf | ||
|
|
||
| # multiclass case: check correct boundary values for eps = 0 | ||
| with pytest.warns(FutureWarning): | ||
| assert log_loss([0, 1, 2], [[1, 0, 0], [0, 1, 0], [0, 0, 1]], eps=0) == 0 | ||
| with pytest.warns(FutureWarning): | ||
| assert ( | ||
| log_loss([0, 1, 2], [[0, 0.5, 0.5], [0, 1, 0], [0, 0, 1]], eps=0) == np.inf | ||
| ) |
There was a problem hiding this comment.
Indeed, I find the following weird:
>>> log_loss([0, 1, 2], [[1, 0, 0], [0, 1, 0], [0, 0, 1]])
2.2204460492503136e-16Shall to conditionally clip to eps only when one_hot_encode(y_true) > 0 and clip to 0 otherwise?
| # binary case: check correct boundary values for eps = 0 | ||
| with pytest.warns(FutureWarning): | ||
| assert log_loss([0, 1], [0, 1], eps=0) == 0 | ||
| with pytest.warns(FutureWarning): | ||
| assert log_loss([0, 1], [0, 0], eps=0) == np.inf | ||
| with pytest.warns(FutureWarning): | ||
| assert log_loss([0, 1], [1, 1], eps=0) == np.inf | ||
|
|
||
| # multiclass case: check correct boundary values for eps = 0 | ||
| with pytest.warns(FutureWarning): | ||
| assert log_loss([0, 1, 2], [[1, 0, 0], [0, 1, 0], [0, 0, 1]], eps=0) == 0 | ||
| with pytest.warns(FutureWarning): | ||
| assert ( | ||
| log_loss([0, 1, 2], [[0, 0.5, 0.5], [0, 1, 0], [0, 0, 1]], eps=0) == np.inf | ||
| ) |
There was a problem hiding this comment.
Actually whatever the decision on this, there should be a test to cover the case where log_loss reaches its minimum (perfect predictions), both in binary and multiclass settings.
Co-authored-by: Guillaume Lemaitre <guillaume@probabl.ai>
epsparam oflog_loss.log_lossnow raises an error when predicted probas do not sum to 1.