-
-
Notifications
You must be signed in to change notification settings - Fork 445
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
Update SmallRng and remove rand_xorshift crate #623
Conversation
Discussion of test failures: #624 |
Updated with a tiny bit of documentation (PRNGs module). We still need to decide what to do with that module; possibly we could move the documentation here (as a new file). For 0.6 the |
I should mention that this was rebased on #632 (which I merged without review since all content other than CI changes had been reviewed elsewhere and the move was already discussed in #624). The changes here have also been discussed elsewhere (#603). More documentation changes may be appropriate; I haven't had time to review that properly. |
src/prng/mod.rs
Outdated
// 2. imperfect performance on tests or other limiting properties, but not | ||
// terrible (e.g. Xoroshiro128+) | ||
// 2. imperfect performance on tests or other limiting properties, or | ||
// insufficient theory, but not terrible (e.g. SFC, Xoshiro, Xoroshiro128+) |
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.
I disagree with the example RNGs here. Maybe just remove them?
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.
This stuff isn't in the printed doc (only two leading slashes). Yeah, it's difficult to know how to rate things; there's enough complexity to write a book on it or we could simply leave users with no hints at all. I already changed this rating system to demote PCG to 3 stars because I don't think it should be compared so closely with crypto RNGs.
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.
Fair enough. The 4 and 5 star ratings are unused anyway, because the CSPRNG sections does not have a quality rating.
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.
It could, but unless all PRNGs were put in a single table it wouldn't make much sense. Further comments on this in #633.
The rest looks good to me. |
This updates
SmallRng
as mentioned in #603.rand_xorshift
has been moved here.