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

Use torch.testing.assert_close in common_utils.py #3873

Merged
merged 28 commits into from
May 22, 2021

Conversation

NicolasHug
Copy link
Member

part of #3865

@@ -139,7 +141,8 @@ def assertExpected(self, output, name, prec=None):
raise RuntimeError("The output for {}, is larger than 50kb".format(filename))
else:
expected = torch.load(expected_file)
self.assertEqual(output, expected, prec=prec)
rtol = atol = prec or self.precision
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for ref self.assertEqual is currently setting prec to self.precision if it's None (as done above)

and the check was

tolerance = prec + prec * abs(a.max())
self.assertLessEqual(max_err, tolerance, message)

So the changes LGTM here

Copy link
Member Author

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't approve as I'm the PR author, but this is @pmeier 's work, LGTM when green

Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stamping

@NicolasHug NicolasHug merged commit 3f556e2 into pytorch:master May 22, 2021
facebook-github-bot pushed a commit that referenced this pull request May 25, 2021
Summary: Co-authored-by: Philip Meier <github.pmeier@posteo.de>

Reviewed By: vincentqb, cpuhrsch

Differential Revision: D28679991

fbshipit-source-id: 46e760f6c01c4a8147f5ec373f682b6da96e295a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants