Skip to content

TST use global_random_seed in sklearn/linear_model/tests/test_linear_loss.py #30863

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

Merged
merged 3 commits into from
Feb 21, 2025

Conversation

sortofamudkip
Copy link
Contributor

Reference Issues/PRs

Towards #22827. All tests passed. @adrinjalali

What does this implement/fix? Explain your changes.

Added global_random_seed parameter to:

  • test_init_zero_coef
  • test_loss_grad_hess_are_the_same (passed to random_X_y_coef)
  • test_loss_gradients_hessp_intercept (passed to random_X_y_coef)
  • test_gradients_hessians_numerically (passed to random_X_y_coef)
  • test_multinomial_coef_shape (passed to random_X_y_coef)
  • test_multinomial_hessian_3_classes (passed to random_X_y_coef)

Copy link

github-actions bot commented Feb 19, 2025

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 581d885. Link to the linter CI: here

@StefanieSenger
Copy link
Contributor

StefanieSenger commented Feb 19, 2025

Hi @sortofamudkip,

thanks for your contribution. In order to properly run all tests, can you please push an empty commit inlcuding all the names of the tests that you have changed, like this:

git commit --allow-empty -m "empty commit to trigger all CI jobs [all random seeds]"
test_1
test_2
test_3
name_of_more_tests

This is mentioned in the issue an d needed to run the tests with all the available seeds.

test_init_zero_coef
test_loss_grad_hess_are_the_same
test_loss_gradients_hessp_intercept
test_gradients_hessians_numerically
test_multinomial_coef_shape
test_multinomial_hessian_3_classes
test_init_zero_coef
test_loss_grad_hess_are_the_same
test_loss_gradients_hessp_intercept
test_gradients_hessians_numerically
test_multinomial_coef_shape
test_multinomial_hessian_3_classes
Copy link
Contributor

@StefanieSenger StefanieSenger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @sortofamudkip, it looks all good to me.

I think we can merge this. Would you mind having a look @ogrisel or @betatim? This is from the sprint we had in Berlin yesterday.

@adrinjalali adrinjalali added Quick Review For PRs that are quick to review Waiting for Second Reviewer First reviewer is done, need a second one! labels Feb 20, 2025
Copy link
Contributor

@OmarManzoor OmarManzoor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @sortofamudkip

@OmarManzoor OmarManzoor merged commit 601dfd1 into scikit-learn:main Feb 21, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:linear_model No Changelog Needed Quick Review For PRs that are quick to review Waiting for Second Reviewer First reviewer is done, need a second one!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants