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
Generate a Gaussian noise dataset #52
Comments
Edited based on Tristan's comment
Is the above correct? |
that's right, except that |
@tristpinsm Thanks! |
Notes from meeting: For a random generator, the following key things are needed:
|
Do we want to update the other tasks to use |
Would the reason to do so be performance? Do we think using |
I will look into the difference between them! I just noticed that @jrs65 recommended using |
yeah I had never heard of |
I think this has been integrated in |
I don't think there's any reason to go back and change things, but as As @tristpinsm says the advantage is performance, and there are certain tasks (of which this will be one) where the speed of the RNG is the bottleneck. I introduced it for the delay power spectrum estimator (which in some sense internally does what your doing here hundreds of times), and it took it down from 40 mins per power spectrum to more like 10 mins. |
I agree that it looks like it has been merged into numpy in 1.17 (or at least the core). I'd like to check that it still has the OpenMP parallel mode though before dropping the dependency. |
Understood! The reason behind changing the original ones was to simplify the seed state setting (to avoid having one for setting the NumPy seed and one for setting the RandomGen seed). But if it is integrated into |
So, seeding seems to work in different ways for the "legacy random" and the "new generators".
The new So if we bump to NumPy 1.17, it will be a bit of a refactor, and the two random generators do not intersect with their seed states. |
I cannot confirm whether it still has OpenMP parallel mode, but I do not see anything in here that would indicate support being removed: numpy/numpy#13163. |
It seems like the changes are pretty significant between 1.16 and 1.17. Should we create an issue to migrate whatever uses the old |
@tristpinsm Agreed, they are big and out of scope for this issue/pr, and I think it would be good to do the migration. |
This task should:
weight
property).RandomGen
which is already a dependency)@tristpinsm feel free to add items to this one.
The text was updated successfully, but these errors were encountered: