Closed
Conversation
Previously, test_out used `OpDTypes.none` and then it pretty much implemented `OpDtypes.any_type` inside. This PR changes it to use `OpDTypes`. This has the advantage that the test now has a dtype, so it can be used together with decorators that require a `dtype`, such as `toleranceOverride`. [ghstack-poisoned]
Contributor
🔗 Helpful links
✅ No Failures (0 Pending)As of commit ccc3a6f (more details on the Dr. CI page): Expand to see more💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
Previously, test_out used `OpDTypes.none` and then it pretty much implemented `OpDtypes.any_type` inside. This PR changes it to use `OpDTypes`. This has the advantage that the test now has a dtype, so it can be used together with decorators that require a `dtype`, such as `toleranceOverride`. [ghstack-poisoned]
mruberry
reviewed
May 18, 2022
| # "safely" cast to | ||
| @ops(_ops_and_refs, dtypes=OpDTypes.none) | ||
| def test_out(self, device, op): | ||
| @ops(_ops_and_refs, dtypes=OpDTypes.any_one) |
mruberry
reviewed
May 18, 2022
| dtypes = {dtype} | ||
| break | ||
| else: | ||
| dtypes = {} |
Collaborator
There was a problem hiding this comment.
When could this case trigger? Should there actually be an assertion that this never happens?
Collaborator
Author
There was a problem hiding this comment.
This preserves the previous behaviour (rather than skipping the test if this happens. But yeah, this could be perhaps an assert(false). We can do it in a follow-up PR
mruberry
approved these changes
May 18, 2022
Collaborator
mruberry
left a comment
There was a problem hiding this comment.
Cool!
Maybe make one case an assert?
facebook-github-bot
pushed a commit
that referenced
this pull request
May 20, 2022
Summary: Previously, test_out used `OpDTypes.none` and then it pretty much implemented `OpDtypes.any_type` inside. This PR changes it to use `OpDTypes`. This has the advantage that the test now has a dtype, so it can be used together with decorators that require a `dtype`, such as `toleranceOverride`. Pull Request resolved: #77735 Approved by: https://github.com/mruberry Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/c446f78ffd1bf2ce39a890f02325991b4441080b Reviewed By: seemethere Differential Revision: D36494178 Pulled By: seemethere fbshipit-source-id: 112590f2e837eaf6861126c789929076b8692e07
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
4 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.
Stack from ghstack:
Previously, test_out used
OpDTypes.noneand then it pretty muchimplemented
OpDtypes.any_typeinside. This PR changes it to useOpDTypes. This has the advantage that the test now has a dtype, so itcan be used together with decorators that require a
dtype, such astoleranceOverride.