TST use global_random_seed in sklearn/metrics/tests/test_regression.py #30865
+23
−17
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
test_tweedie_deviance_continuity test_mean_absolute_percentage_error test_mean_pinball_loss_on_constant_predictions test_dummy_quantile_parameter_tuning test_pinball_loss_relation_with_mae
NMM3N
approved these changes
Mar 6, 2025
NMM3N
approved these changes
Mar 6, 2025
OmarManzoor
reviewed
Mar 13, 2025
| assert result.success | ||
| # The minimum is not unique with limited data, hence the large tolerance. | ||
| assert result.x == pytest.approx(best_pred, rel=1e-2) | ||
| assert result_x == pytest.approx(best_pred, rel=1e-2) |
There was a problem hiding this comment.
I think the issue is mostly with recovering the actual value which can turn out to be somewhat different than the actual best_pred. Moreover the optimization might also not be converging to the actual minimum loss on some occasions using the default tolerance in the minimize method.
What would be the best approach to handling this @ogrisel @lorentzenchr
There was a problem hiding this comment.
Given the approximate nature of this part of the test due to the limited data, increasing the tol looks fine to me. I also added an absolute tol as well to take into account the normal distribution + 0.5 quantile (i.e. expected close to 0).
test_tweedie_deviance_continuity test_mean_absolute_percentage_error test_mean_pinball_loss_on_constant_predictions test_dummy_quantile_parameter_tuning test_pinball_loss_relation_with_mae
jeremiedbb
approved these changes
Apr 11, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference Issues/PRs
Towards #22827.
What does this implement/fix? Explain your changes.
The modified tests are:
test_tweedie_deviance_continuity: tolerance was changed from 1e-6 to 1e-5 to allow all tests to pass.test_mean_absolute_percentage_error: all tests passed.test_dummy_quantile_parameter_tuning: all tests passed.test_pinball_loss_relation_with_mae: all tests passed.test_mean_pinball_loss_on_constant_predictions: some tests failed.In particular, tests failed locally with the following configurations:
n_samples=3000n_samples=30000distribution=uniformdistribution=exponentialdistribution=normalAll tests for
distribution=lognormalpassed.Increasing the number of samples causes
distribution=normalto fail more often.