-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[PRNG] Add check to PRNG to make sure that unsigned integer arithmetic is wrapping #7287
Conversation
Thanks @tkonolige. One thing to note that assert may not be available in every platform, better to put down a comment. Might be useful to have it as a separate testing function. |
e6bd9d3
to
7d8237a
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, I'd prefer if we could have the test function defined in the test file since it doesn't depend on any TOPI PRNG internals but not a huge deal
bump @tqchen @ZihengJiang I think this is good to go since we resolved the assert discussion |
Merged. Thanks @tkonolige @altanh |
…c is wrapping (apache#7287) * [PRNG] Add check to PRNG to make sure that unsigned integer arithmetic is wrapping * Add threefry_test_wrapping: a manual test for wrapping unsigned arithmetic. * fix test to actually run on the target * formatting * lint
…c is wrapping (apache#7287) * [PRNG] Add check to PRNG to make sure that unsigned integer arithmetic is wrapping * Add threefry_test_wrapping: a manual test for wrapping unsigned arithmetic. * fix test to actually run on the target * formatting * lint
…c is wrapping (apache#7287) * [PRNG] Add check to PRNG to make sure that unsigned integer arithmetic is wrapping * Add threefry_test_wrapping: a manual test for wrapping unsigned arithmetic. * fix test to actually run on the target * formatting * lint
…c is wrapping (apache#7287) * [PRNG] Add check to PRNG to make sure that unsigned integer arithmetic is wrapping * Add threefry_test_wrapping: a manual test for wrapping unsigned arithmetic. * fix test to actually run on the target * formatting * lint
This PR adds a simple test to
threefry_generate
to make sure that unsigned integer arithmetic is wrapping. We have no guarantee of wrapping on our platforms, so this is the best we can do. I've added the test tothreefry_generate
as it should be called before other threefry functions and it should called infrequently.@altanh @antinucleon @junrushao1994 @eric-haibin-lin @MarisaKirisame