Skip to content

Conversation

@MagnusBuehler
Copy link
Contributor

@MagnusBuehler MagnusBuehler commented Sep 16, 2025

Defines a session-scoped fixture in conftest.py that sets seeds for torch, numpy, and random to ensure deterministic test behavior across sessions.

Motivation and Context

Multiple tests rely on randomly sampled data using random, numpy, and torch. Without a fixed seed, this can lead to non-deterministic and flaky test results. Adding a global seed fixture improves consistent and reproducible test behavior across runs.

Public API Changes

  • No Public API changes
  • Yes, Public API changes (Details below)

How Has This Been Tested?

On a local machine the randomly sampled data across test runs are consistent.

Checklist

  • The changes have been tested locally.
  • Documentation has been updated (if the public API or usage changes).
  • A entry has been added to CHANGELOG.md (if relevant for users).
  • The code follows the project's style guidelines.
  • I have considered the impact of these changes on the public API.

Defines a session-scoped fixture in conftest.py that sets seeds for torch, numpy, and random to ensure deterministic test behavior across sessions.
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a global pytest fixture to set random seeds for torch, numpy, and random, aiming to improve test reproducibility. The implementation is sound and correctly uses a session-scoped, autouse fixture. My feedback includes suggestions to further enhance determinism for CUDA operations, add a docstring for clarity, and improve comments within the fixture.

@noahho noahho requested review from noahho and oscarkey September 19, 2025 09:40
Copy link
Collaborator

@noahho noahho left a comment

Choose a reason for hiding this comment

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

I think this is a very positive change, hope I'm not overlooking anything here. Thanks a lot! (let's wait for Oscars review too)

Copy link
Contributor

@oscarkey oscarkey left a comment

Choose a reason for hiding this comment

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

This seems like a good idea to me! Thank you. Just a few small suggestions.

- removed unnecessary comment
- fixture scope = "function"
- return type
- removed unnecessary torch.cuda.manual_seed_all call
@noahho noahho merged commit 0a91b1d into PriorLabs:main Oct 2, 2025
10 checks passed
oscarkey pushed a commit that referenced this pull request Nov 12, 2025
* Record copied public PR 516

* Add global seed fixture for reproducible tests (#516)

Co-authored-by: MagnumMandel <111045718+PufferFishCode@users.noreply.github.com>
Co-authored-by: noahho <Noah.homa@gmail.com>

---------

Co-authored-by: mirror-bot <mirror-bot@users.noreply.github.com>
Co-authored-by: MagnusBuehler <111045718+MagnusBuehler@users.noreply.github.com>
Co-authored-by: MagnumMandel <111045718+PufferFishCode@users.noreply.github.com>
Co-authored-by: noahho <Noah.homa@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants