-
Notifications
You must be signed in to change notification settings - Fork 539
Fixes Non-Deterministic Tests #1295
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
Fixes Non-Deterministic Tests #1295
Conversation
|
/blossom-ci |
Greptile SummaryFixed non-deterministic test behavior in
Important Files Changed
|
Greptile's behavior is changing!From now on, if a review finishes with no comments, we will not post an additional "statistics" comment to confirm that our review found nothing to comment on. However, you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
…m/peterdsharpe/physicsnemo into psharpe/fix-nondeterministic-test
… in various test files.
|
/blossom-ci |
|
Actually a dumb question: would it work if we just had a top level # not sure if function, module, or session would be better
@pytest.fixture(autouse=True, scope="module")
def set_random_seed():
SEED = 3519581
torch.manual_seed(SEED)
random.seed(SEED)
np.random.seed(SEED) # use legacy global state
yieldIMO it does lift the burden from making sure individual (and new) tests are seeded so this doesn't become an issue That being said though, tests that are failing because of unset seeds could be an issue with the test itself... |
|
@laserkelvin Never a dumb question! I actually like your proposal a lot - seems like a better way to prevent this mistake in the future. I'll have to do some digging to see if it's technically feasible, but if it is, I'll implement it instead. |
|
/blossom-ci |
I'm prepared to delete my comment if it doesn't work 😆 |
laserkelvin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for humoring my suggestion!
|
/blossom-ci |
…rministic behavior is handled globally, improving test reliability.
…eterministic behavior is handled globally, improving test reliability.
|
/blossom-ci |
|
/blossom-ci |
PhysicsNeMo Pull Request
Description
On rare occasion, hitting sporadic CI failure due to unseeded randomness in tests.
Checklist
Dependencies
Review Process
All PRs are reviewed by the PhysicsNeMo team before merging.
Depending on which files are changed, GitHub may automatically assign a maintainer for review.
We are also testing AI-based code review tools (e.g., Greptile), which may add automated comments with a confidence score.
This score reflects the AI’s assessment of merge readiness and is not a qualitative judgment of your work, nor is
it an indication that the PR will be accepted / rejected.
AI-generated feedback should be reviewed critically for usefulness.
You are not required to respond to every AI comment, but they are intended to help both authors and reviewers.
Please react to Greptile comments with 👍 or 👎 to provide feedback on their accuracy.