Skip to content
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 for user-input random seed #164

Closed
arjunsavel opened this issue Oct 19, 2020 · 3 comments
Closed

Allow for user-input random seed #164

arjunsavel opened this issue Oct 19, 2020 · 3 comments
Assignees

Comments

@arjunsavel
Copy link

There are a few modules (pulsar.pulsar, utils.utils, telescope.receiver) where either the numpy.random module is explicitly called or samples are drawn from a normal distribution. In the interest of reproducibility (or exploring the effects of random noise on final results), I recommend allowing for user-input random seeds to initialize any random processes used. I would not consider implementation of this necessary for JOSS acceptance, but I would recommend a brief acknowledgment of plots in documentation not being exactly reproducible due to random processes.

This issue is part of my JOSS review.

@paulthebaker
Copy link
Contributor

Because scipy.stats uses numpy.random under the hood, a user can specify a random seed by calling numpy.random.seed() before running a function that uses a random number generator (RNG). We should add this fact to the documentation as part of the JOSS revisions.

This is a legacy way to seed the RNG. There's a newer way to do this as of Numpy 1.17 using BitGenerator objects, but the old way will still work for a while. One design motivation of the new way is to provide reproducible random samples across platforms for unit testing. For compatibility we might want to allow users to pass in integer seeds, older RandomState instances, or newer BitGenerator instances.

There are two places where we use a RNG, pulse generation (pulsar._make_amp_pulses() and _make_pow_pulses()) and noise generation (reciever.radiometer_noise()). I could imagine someone might want to use identical pulses in new noise or keep the exact same noise but simulate new pulses. In which case there should be at least three places to provide seeds: pulsar.make_pulses(), telescope.observe(), and a global seed (for an easy way to reseed a simulation).

Implementing seeding the "right" way is beyond the scope of the JOSS review, but we should do it eventually. Getting the desired RNG infrastructure in place, will give an example for how contributors should implement RNG methods for other use cases.

@Hazboun6
Copy link
Contributor

@paulthebaker Am I interpreting this correctly to understand that, at least in regards to the JOSS review, this is in my court? I'm working on the docs, so this is easy for me to do.

@Hazboun6
Copy link
Contributor

I added the following Markdown cell at the end of each tutorial:

Note about randomly generated pulses and noise

PsrSigSim uses numpy.random under the hood in order to generate
the radio pulses and various types of noise. If a user desires or
requires that this randomly generated data is reproducible we recommend
using a call the seed generator native to Numpy before calling the
function that produces the random noise/pulses. Newer versions of
Numpy are moving toward slightly different
functionality/syntax,
but is essentially used in the same way.

numpy.random.seed(1776)
pulsar_1.make_pulses(signal_1, tobs=obslen)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants