The Wayback Machine - https://web.archive.org/web/20211030204711/https://github.com/pytorch/pytorch/pull/63571
Skip to content
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

Jump to bottom

enable equal_nan for complex values in isclose #63571

Closed
wants to merge 2 commits into from
Closed

enable equal_nan for complex values in isclose #63571

wants to merge 2 commits into from

Conversation

Reviewers

@mruberry mruberry

Assignees
No one assigned
Labels
cla signed Merged module: testing open source
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@pmeier @facebook-github-bot @mruberry @pytorchbot
@pmeier
Copy link
Collaborator

@pmeier pmeier commented Aug 19, 2021 •
edited by mruberry

Stack from ghstack:

Differential Revision: D30560127
@pmeier
enable equal_nan for complex values in isclose
8574d79 
[ghstack-poisoned]
pmeier added a commit that referenced this issue Aug 19, 2021
@pmeier
enable equal_nan for complex values in isclose
d199c26 
ghstack-source-id: 3bbc12174311b6cac6628a047b8fdd018823a471
Pull Request resolved: #63571
@facebook-github-bot
Copy link
Contributor

@facebook-github-bot facebook-github-bot commented Aug 19, 2021 •
edited

🔗 Helpful links

💊 CI failures summary and remediations

As of commit 18ab90e (more details on the Dr. CI page):

  • 1/1 failures possibly* introduced in this PR
    • 1/1 non-scanned failure(s)

ci.pytorch.org: 1 failed

This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@pmeier pmeier mentioned this pull request Aug 19, 2021
Closed
pmeier
pmeier reviewed Aug 19, 2021
View changes
aten/src/ATen/native/TensorCompare.cpp
if (equal_nan && self.is_floating_point()) {
close.__ior__((self != self).__iand__(other != other));
if (equal_nan && (self.is_floating_point() || self.is_complex())) {
close.__ior__(self.isnan().__iand__(other.isnan()));
Copy link
Collaborator Author

@pmeier pmeier Aug 19, 2021

Although it is functionally equal, self.isnan() is a lot more legible than self != self

Copy link
Contributor

@mruberry mruberry Aug 25, 2021

I guess so. The compiler can probably optimize away the extra function call.

test/test_testing.py
tests = (
(complex(1, 1), complex(1, float('nan')), False),
(complex(1, 1), complex(float('nan'), 1), False),
(complex(float('nan'), 1), complex(float('nan'), 1), True),
(complex(float('nan'), 1), complex(1, float('nan')), False),
(complex(float('nan'), float('nan')), complex(float('nan'), float('nan')), True),
)
Copy link
Collaborator Author

@pmeier pmeier Aug 19, 2021

The test cases will be merged into the ones above later in the stack (#63572), when TestCase.assertEqual stops treating complex components separately.

test/test_testing.py
if dtype.is_floating_point:
a = b = torch.nan
else:
a = complex(torch.nan, 0)
b = complex(0, torch.nan)
Copy link
Collaborator Author

@pmeier pmeier Aug 19, 2021

We can also split this into two tests for float and complex.

@pmeier pmeier requested a review from mruberry Aug 19, 2021
@pmeier
Update on "enable equal_nan for complex values in isclose"
18ab90e 
[ghstack-poisoned]
@pmeier pmeier mentioned this pull request Aug 24, 2021
Closed
mruberry
mruberry approved these changes Aug 25, 2021
View changes
Copy link
Contributor

@mruberry mruberry left a comment

Cool! And it looks like the docs don't need to be updated, either.

@mruberry
Copy link
Contributor

@mruberry mruberry commented Aug 25, 2021

@mruberry has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@facebook-github-bot facebook-github-bot commented Aug 26, 2021

@mruberry merged this pull request in b1154cc.

@pmeier pmeier deleted the gh/pmeier/34/head branch Aug 26, 2021
cyyever added a commit to cyyever/pytorch that referenced this issue Aug 30, 2021
@pmeier @cyyever
enable equal_nan for complex values in isclose (pytorch#63571)
a99dbd7 
Summary: Pull Request resolved: pytorch#63571

Test Plan: Imported from OSS

Reviewed By: malfet, ngimel

Differential Revision: D30560127

Pulled By: mruberry

fbshipit-source-id: 8958121ca24e7c139d869607903aebbe87bc0740
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment