-
Notifications
You must be signed in to change notification settings - Fork 431
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
Implemented XoShiro128 psuedo random number generator. #428
Conversation
Hi @thelearnerofcode. Thank you for the PR! We have plans to move all specific PRNG implementations out of Rand, so I don't think adding any xoshiro variants is what we want at the moment. And we have a couple of other ones higher up the list in terms of priority, which are already postponed until after Rand 0.5 gets released. I have not read the details of this new RNG yet, it is only released very recently. Maybe it should prove itself for a couple of months. And I am curious how it holds up under TestU01 and PractRand, where Xoroshiro128+ performed worse than what was advertised. |
Ok! Should I close the PR? |
Maybe for now, but you can also leave it open for some discussion. I am sure @vks is interested, he mentioned the same RNG yesterday in dhardy#60 (comment), and is the author of the xoroshiro crate, which implements more work by Vigna. |
I worked on a crate for xoshiro128** yesterday. It can be partially vectorized with SIMD, resulting in a nice speedup. |
Cool! Sounds to me like we should start discussing how we want to present extra RNGs to users (dhardy#58) soon (in fact, if someone wants to open a new issue with a proposal, go ahead; I think companion crates are probably the best option but don't know how many). However, I don't think we should merge anything before the 0.5 release (very soon, hopefully). This PR looks very nice but I don't think we should merge new PRNGs before sorting out the above. Also since this is part of a family of new PRNGs I don't know if we should pick names like PS nice SIMD work @vks. But how are we supposed to differentiate between |
@dhardy Not sure what you mean. There is only one xoshiro128** and xoshiro128+ version, both use |
This PR does not implement xoshiro128** as claimed by the code, but rather xoroshiro128**! |
Lets close this for now. We don't want it in its current form, and if we do decide to add this PRNG we can refer back to this PR. |
Still have to add tests and documentation, but this is a good start.
See here for more info: http://xoshiro.di.unimi.it/.