-
-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Local testing of global_random_seed is not enough #28959
Labels
Comments
jeremiedbb
added
Numerical Stability
and removed
Needs Triage
Issue requires triage
labels
May 6, 2024
@jeremiedbb I would like to contribute to this, but am new to CI. To improve this issue, my understanding is to add [all random seeds] in tests in .yml files in each CI. Is that correct? |
Hi @kyrajeep, there's no action needed regarding this issue, it's mostly a reminder for reviewers. |
Thank you for letting me know! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When adding
global_random_seed
to a test, it's not enough to check it locally, i.e. on a single machine. Numerical precision issues can come from various factors like OS, CPU, BLAS, ...When adding
global_random_seed
, it's important to test all random seeds on all CI jobs. To do that, you need to push a commit with[all random seeds]
and the list of tests to check in the commit message:If this is not done, we merge the PR and then the nightly builds fail every once in a while because the tolerance was barely too small for some seed.
The text was updated successfully, but these errors were encountered: