-
Notifications
You must be signed in to change notification settings - Fork 41
add random seed generation #410
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
Conversation
Anton-4
left a comment
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.
Great PR @jfmontanaro :)
I left some minor comments, let's add a random_seed_u32! as well.
|
Thanks, will do! Unfortunately I'm out of time for now, so this might have to wait until this evening or tomorrow, but it shouldn't take long once I can get to it. I see the CI is failing, it looks like it's looking for Should I just go ahead and add a |
|
Ok, I think this is ready for review. I added Doc comments now a) link to the |
|
Thanks for the edits @jfmontanaro! I made some final changes:
|
|
Sounds good, thanks! |
As discussed on Zulip, this PR adds an API to generate a random seed.
It uses the Rust crate
getrandom- originally I was going to userand, but I realized thatrandusesgetrandomunder the hood anyway, so there's no need for an extra layer of indirection.I decided to just stick with a single
seed!()function for now that generates a U64. I figure if you want differently-shaped data you can use a library like roc-random and just use this to seed it.I didn't realize until after I put it all together that
roc_randomuses 32-bit integers for its seeds, rather than 64-bit. I went with 64 bits because I figured, more randomness better, right? But I'd be happy to change it, or add a variant, if that would be better.I also realize that 64 bits of entropy isn't generally considered enough for some applications (especially cryptographic ones), but I didn't know if it was worth providing an interface to e.g.
getrandom::fil()or anything like that. Again, happy to add/change whatever seems best!