Fix integer overflow in remainder operator#5906
Merged
soumith merged 4 commits intopytorch:masterfrom Mar 21, 2018
vedanuj:fix_int_overflow
Merged
Fix integer overflow in remainder operator#5906soumith merged 4 commits intopytorch:masterfrom vedanuj:fix_int_overflow
soumith merged 4 commits intopytorch:masterfrom
vedanuj:fix_int_overflow
Conversation
Contributor
|
Nice catch, @vedanuj! From the code changes, it looks like integer overflow from the multiplication is causing the issue. Is this correct? In the issue, the @robert-wagner mentioned two failure cases. Let
I listed an example of the first failure case in the issue; do you have an example of the second failure case, @robert-wagner? |
Contributor
|
Hmm also see the failing tests (hit Details). |
Contributor
Author
|
@zou3519 Yes the integer overflow was causing the issue. I will add the tests and check why the test cases are failing. |
Contributor
|
x = torch.tensor(-23500, dtype=torch.int64) |
Contributor
Author
|
The new commits fix the remainder operator in Cuda. Add tests in Tests : |
zou3519
reviewed
Mar 20, 2018
Contributor
zou3519
left a comment
There was a problem hiding this comment.
This looks great! One minor comment
| // There is no NAN for integers | ||
| rp[i] = tp[i] % value; | ||
| if (rp[i] * value < 0) | ||
| if ((rp[i] < 0) != (value < 0)) |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Contributor
Author
|
@pytorchbot retest this please |
jekbradbury
pushed a commit
to jekbradbury/pytorch
that referenced
this pull request
Mar 21, 2018
* Fix integer overflow in remainder * Fix remainder operator in CUDA * Add tests for remainder integer overflow * Add has_different_sign static function
jfix71
added a commit
to jfix71/pytorch
that referenced
this pull request
Jan 31, 2022
…#5906) Summary: Pull Request resolved: pytorch/glow#5906 Pull Request resolved: pytorch#71883 Fixes slice_tensor retracing. Include fix for retrace coverage. Missed in D33760455 (pytorch@66939e3). Test Plan: CI Reviewed By: wushirong Differential Revision: D33802222 fbshipit-source-id: 6796d65ea79107c0fcc577b583601f7c2dd35db1
facebook-github-bot
pushed a commit
that referenced
this pull request
Jan 31, 2022
Summary: Pull Request resolved: pytorch/glow#5906 Pull Request resolved: #71883 Fixes slice_tensor retracing. Include fix for retrace coverage. Missed in D33760455 (66939e3). Test Plan: CI Reviewed By: wushirong Differential Revision: D33802222 fbshipit-source-id: 4e0e44ae4a4eb70b99d79f0cd582182031b87e25
pytorchmergebot
pushed a commit
that referenced
this pull request
Jan 31, 2022
Summary: Pull Request resolved: pytorch/glow#5906 Pull Request resolved: #71883 Fixes slice_tensor retracing. Include fix for retrace coverage. Missed in D33760455 (66939e3). Test Plan: CI Reviewed By: wushirong Differential Revision: D33802222 fbshipit-source-id: 4e0e44ae4a4eb70b99d79f0cd582182031b87e25 (cherry picked from commit 98fd23c)
laurentdupin
pushed a commit
to laurentdupin/pytorch
that referenced
this pull request
Apr 24, 2026
* Fix integer overflow in remainder * Fix remainder operator in CUDA * Add tests for remainder integer overflow * Add has_different_sign static function
laurentdupin
pushed a commit
to laurentdupin/pytorch
that referenced
this pull request
Apr 25, 2026
…#5906) Summary: Pull Request resolved: pytorch/glow#5906 Pull Request resolved: pytorch#71883 Fixes slice_tensor retracing. Include fix for retrace coverage. Missed in D33760455 (pytorch@286a259). Test Plan: CI Reviewed By: wushirong Differential Revision: D33802222 fbshipit-source-id: 4e0e44ae4a4eb70b99d79f0cd582182031b87e25 (cherry picked from commit 98fd23c)
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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 fixes issue #5875 . Fixes integer overflow due to multiplication.
Please review @zou3519