-
Notifications
You must be signed in to change notification settings - Fork 258
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
Allow to use random seed in make_synthetic_data #1286
Conversation
Add a new `random_seed` argument to the `make_synthetic_data` method. Switch to the Numpy's random number generator object and pass the `random_seed` argument to `np.random.default_rng`, allowing users to specify a random seed for the synthetic noise.
Replace calls to `np.random.rand` in some tests functions that were failing locally for calls to methods of Numpy's random number generator, defined with a hard-coded seed. It would be ideal to make these changes in the whole test suite.
Codecov Report
@@ Coverage Diff @@
## main #1286 +/- ##
=======================================
Coverage 82.36% 82.36%
=======================================
Files 164 164
Lines 25098 25099 +1
=======================================
+ Hits 20673 20674 +1
Misses 4425 4425
|
@jcapriot I managed to get the tests to pass here. But at some point I would love to see all tests run with a hard coded seed. Do you think it worth tackling that in this PR or should we merge this one and tackle the rest in a separate one? |
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.
Thanks @santisoler! It will be valuable to have better practices in place for dealing with randomness. This looks good to me 🚀
I know there are a lot of other places across SimPEG to do this, one of the higher priority places to look to next would be the directives: I am specifically thinking of the beta-estimator. We can start an issue if you like, I am just commenting here as it is top-of-mind as I look at these updates
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Summary
Add a new
random_seed
argument to themake_synthetic_data
method.Switch to the Numpy's random number generator object and pass the
random_seed
argument tonp.random.default_rng
, allowing users tospecify a random seed for the synthetic noise. Set random seeds in some
tests that were failing due to the new random state that the usage of
make_synthetic_data
withrandom_seed
created in the test suite.PR Checklist
expect style.
to a Pull Request
@simpeg/simpeg-developers
when ready for review.Reference issue
Related to #1289
What does this implement/fix?
Additional information