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 test_anchor_utils.py #3880

Merged
merged 25 commits into from
May 21, 2021

Conversation

NicolasHug
Copy link
Member

part of #3865

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.

Minor comment but it's more about assert_close than about the proposed changes in this PR, LGTM

Comment on lines +89 to +90
torch.testing.assert_close(dboxes[0], dboxes_output, rtol=1e-5, atol=1e-8)
torch.testing.assert_close(dboxes[1], dboxes_output, rtol=1e-5, atol=1e-8)
Copy link
Member Author

Choose a reason for hiding this comment

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

Here it's a bit of a shame IMHO that we're forced to specify atol as well. The default for atol is 1e-5 and

        torch.testing.assert_close(dboxes[0], dboxes_output, rtol=1e-5, atol=1e-5)
        torch.testing.assert_close(dboxes[1], dboxes_output, rtol=1e-5, atol=1e-5)

passes just fine, so ideally we could just write

        torch.testing.assert_close(dboxes[0], dboxes_output, rtol=1e-5)
        torch.testing.assert_close(dboxes[1], dboxes_output, rtol=1e-5)

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 86d4541 into pytorch:master May 21, 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: D28679964

fbshipit-source-id: c53eb63031a4b965e870f3334d749e4d9d41ddad
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