-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
[v1.5.0] Fix handling of non-finite values in topk (#35253) #35435
Conversation
Summary: Fixes pytorch#34191 `at::native::radixSelect` basically uses integer comparison which creates a defined ordering of non-finite float values. This isn't compatible with IEEE float comparison, so mixing the two leads to unwritten values in the output. Pull Request resolved: pytorch#35253 Differential Revision: D20645554 Pulled By: ezyang fbshipit-source-id: 651bcb1742ed67086ec89cc318d862caae65b981
💊 CircleCI build failures summary and remediationsAs of commit 71c44bf (more details on the Dr. CI page):
🕵️ 4 new failures recognized by patternsThe following build failures do not appear to be due to upstream breakages (reran 3 jobs to discount flakiness): pytorch_xla_linux_xenial_py3_6_clang7_test (1/4)Step: "Test" (full log | pattern match details)
|
pytorch/xla#1824 might fix that, though we will have to ask @jysohn23 to cherrypick it into our release branch as well. |
@dlibenzi thanks for the pointer. Should I go ahead and land this and then @jysohn23 can cherry-pick it on your side? I'm a little unclear on how quickly that can happen. I guess an alternative is to make up a fake xla branch to point to for testing this PR so we have some confidence before we cherry-pick. What do you think? |
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.
I think this should be good to go, I don't think the testing failures are related to the contents of this PR.
@dlibenzi @ailzhang looks like this is still failing, https://dr.pytorch.org/api/view-log-full?build_id=110804382. Any other ideas? |
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.
@gchanan All good now :D I forgot that we have separate jobs for build and test so I only ran the test job...
Summary:
Fixes #34191
at::native::radixSelect
basically uses integer comparison which creates a defined ordering of non-finite float values. This isn't compatible with IEEE float comparison, so mixing the two leads to unwritten values in the output.Pull Request resolved: #35253
Differential Revision: D20645554
Pulled By: ezyang
fbshipit-source-id: 651bcb1742ed67086ec89cc318d862caae65b981