[PRNG] Add check to PRNG to make sure that unsigned integer arithmetic is wrapping #7287
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
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. |
tkonolige
force-pushed
the
wrapping_test
branch
from
e6bd9d3 to
7d8237a
Compare
altanh
suggested changes
Jan 15, 2021
Contributor
altanh
left a comment
altanh
left a comment
There was a problem hiding this comment.
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
altanh
reviewed
Jan 19, 2021
altanh
approved these changes
Jan 27, 2021
Contributor
|
bump @tqchen @ZihengJiang I think this is good to go since we resolved the assert discussion |
ZihengJiang
approved these changes
Jan 27, 2021
Contributor
|
Merged. Thanks @tkonolige @altanh |
alexwong
pushed a commit
to alexwong/tvm
that referenced
this pull request
Feb 11, 2021
…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
electriclilies
pushed a commit
to electriclilies/tvm
that referenced
this pull request
Feb 18, 2021
…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
Lokiiiiii
pushed a commit
to Lokiiiiii/tvm
that referenced
this pull request
Mar 2, 2021
…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
trevor-m
pushed a commit
to neo-ai/tvm
that referenced
this pull request
Mar 2, 2021
…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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a simple test to
threefry_generateto 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_generateas it should be called before other threefry functions and it should called infrequently.@altanh @antinucleon @junrushao1994 @eric-haibin-lin @MarisaKirisame