Add forward AD test for op info#57701
Closed
albanD wants to merge 9 commits intogh/albanD/88/basefrom
Closed
Conversation
[ghstack-poisoned]
Contributor
💊 CI failures summary and remediationsAs of commit 3a9e11b (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 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. |
The new OpInfo flag has the following semantic: - If not specified, that means the given function does not have forward AD implemented. And we check that the corresponding error is raised - If it says that it supports forward AD, we run gradcheck with forward AD to ensure it is correct - If it says that it does not support it, we skip the test altogether (this happen right now because some formulas are wrong for complex and because the forward AD code make complex input invalid for some linalg functions) [ghstack-poisoned]
The new OpInfo flag has the following semantic: - If not specified, that means the given function does not have forward AD implemented. And we check that the corresponding error is raised - If it says that it supports forward AD, we run gradcheck with forward AD to ensure it is correct - If it says that it does not support it, we skip the test altogether (this happen right now because some formulas are wrong for complex and because the forward AD code make complex input invalid for some linalg functions) [ghstack-poisoned]
This was referenced May 6, 2021
The new OpInfo flag has the following semantic: - If not specified, that means the given function does not have forward AD implemented. And we check that the corresponding error is raised - If it says that it supports forward AD, we run gradcheck with forward AD to ensure it is correct - If it says that it does not support it, we skip the test altogether (this happen right now because some formulas are wrong for complex and because the forward AD code make complex input invalid for some linalg functions) [ghstack-poisoned]
The new OpInfo flag has the following semantic: - If not specified, that means the given function does not have forward AD implemented. And we check that the corresponding error is raised - If it says that it supports forward AD, we run gradcheck with forward AD to ensure it is correct - If it says that it does not support it, we skip the test altogether (this happen right now because some formulas are wrong for complex and because the forward AD code make complex input invalid for some linalg functions) [ghstack-poisoned]
The new OpInfo flag has the following semantic: - If not specified, that means the given function does not have forward AD implemented. And we check that the corresponding error is raised - If it says that it supports forward AD, we run gradcheck with forward AD to ensure it is correct - If it says that it does not support it, we skip the test altogether (this happen right now because some formulas are wrong for complex and because the forward AD code make complex input invalid for some linalg functions) [ghstack-poisoned]
dgl-intel
pushed a commit
to dgl-intel/pytorch
that referenced
this pull request
May 7, 2021
ghstack-source-id: d41fe33 Pull Request resolved: pytorch#57701
mruberry
reviewed
May 11, 2021
| if op.supports_forward_ad: | ||
| self._grad_test_helper(device, dtype, op, op.get_op(), check_forward_ad=True) | ||
| else: | ||
| err_msg = r"Trying to use forward AD with .* that does not support it\." |
mruberry
approved these changes
May 11, 2021
Collaborator
mruberry
left a comment
There was a problem hiding this comment.
This is a brilliant extension of our automated testing
The new OpInfo flag has the following semantic: - If it says that it supports forward AD, we run gradcheck with forward AD to ensure it is correct - If it says that it does not support it, we check that the corresponding error is raised All the added tests take 3s to run for CPU builds and 1min for GPU builds which should be pretty negligible compared to the test_ops runtime for each of these arch. [ghstack-poisoned]
albanD
added a commit
to albanD/pytorch
that referenced
this pull request
May 11, 2021
ghstack-source-id: 541f279 Pull Request resolved: pytorch#57701
The new OpInfo flag has the following semantic: - If it says that it supports forward AD, we run gradcheck with forward AD to ensure it is correct - If it says that it does not support it, we check that the corresponding error is raised All the added tests take 3s to run for CPU builds and 1min for GPU builds which should be pretty negligible compared to the test_ops runtime for each of these arch. [ghstack-poisoned]
The new OpInfo flag has the following semantic: - If it says that it supports forward AD, we run gradcheck with forward AD to ensure it is correct - If it says that it does not support it, we check that the corresponding error is raised All the added tests take 3s to run for CPU builds and 1min for GPU builds which should be pretty negligible compared to the test_ops runtime for each of these arch. [ghstack-poisoned]
Collaborator
Author
|
@albanD has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Contributor
Contributor
|
This pull request has been reverted by f88297c. |
krshrimali
pushed a commit
to krshrimali/pytorch
that referenced
this pull request
May 19, 2021
Summary: Pull Request resolved: pytorch#57701 The new OpInfo flag has the following semantic: - If it says that it supports forward AD, we run gradcheck with forward AD to ensure it is correct - If it says that it does not support it, we check that the corresponding error is raised All the added tests take 3s to run for CPU builds and 1min for GPU builds which should be pretty negligible compared to the test_ops runtime for each of these arch. Test Plan: Imported from OSS Reviewed By: agolynski Differential Revision: D28387767 Pulled By: albanD fbshipit-source-id: 369d76921c8460aa4548f9b5159b7297994672f5
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
3 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.
The new OpInfo flag has the following semantic:
All the added tests take 3s to run for CPU builds and 1min for GPU builds which should be pretty negligible compared to the test_ops runtime for each of these arch.
Stack from ghstack:
Differential Revision: D28387767