-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Metal pseudo random number generation #1533
Conversation
…lakstad/metal-prng
There's a big issue with this implementation: You never update the seed, meaning all tensors will be generated the exact same, this is not OK, we definitely need update the seed from the output of the kernels. Ideally we wouldn't require to sync and just put a real buffer on the device containing the seed (modifying the API in candle-metal-kernels, to force users to provide a buffer instead of a real cpu number). For the entropy, I'm getting similar numbers as the CPU generated numbers, therefore even though 7 seems quite low I think it's ok. (I got 7.3 by generating for the entire f32 range, saving to safetensors and calling |
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.
Looking better
* set_seed via buffer content pointer copy + did_modify_range * ensure random.metal kernel does not write outside of buffer range when tid==0
13dfe29
to
db92351
Compare
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.
LGTM
Hybrid Tausworthe and LCG random number generator, using Box-Muller transform for gaussian normal distribution.
Paper
Cuda ref
Benchmarks:
Randomness verified with ent.
Chi square is obviously not good enough for cryptography, but I think it's good enough for ML.
Uniform:
Normal: