Skip to content

Audit Codebase for Unseeded Random Number Generation #548

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

Open
coreyostrove opened this issue Feb 25, 2025 · 1 comment
Open

Audit Codebase for Unseeded Random Number Generation #548

coreyostrove opened this issue Feb 25, 2025 · 1 comment
Assignees
Milestone

Comments

@coreyostrove
Copy link
Contributor

coreyostrove commented Feb 25, 2025

As mentioned in recent PR #547, there are some places in the codebase where random sampling is performed by seeding either wasn't included, or had incomplete plumbing. This is a problem for cases where deterministically random number generation is needed for exact reproducibility (either in testing or manuscript preparation). There are undoubtedly other places where we've missed this scattered throughout the code base and we should audit it and correct as needed.

I'd also like to propose the following addition to our unofficial PR review guidelines (our "zen of pyGSTi"):

  • If your code uses any RNG it must include the option (and requisite plumbing) to specify a user-specified seed value for deterministically random results. If deterministically random results can't be reasonably guaranteed even with seeding (e.g. even with deterministic RNG interactions with other platform specific implementation details may result in different results on different machines) and you know this to be the case you should include notes in the documentation to that effect.
@coreyostrove coreyostrove added the bug A bug or regression label Feb 25, 2025
@coreyostrove coreyostrove removed the bug A bug or regression label Feb 25, 2025
@sserita sserita added this to the 0.9.15+ milestone Mar 18, 2025
@sserita
Copy link
Contributor

sserita commented Mar 19, 2025

We should also use this audit to migrate from the old NumPy RandState to the new default_rng.

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

No branches or pull requests

4 participants