Fix auto exponent issue for torch.pow #47024
Closed
Conversation
This file contains 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
[ghstack-poisoned]
[ghstack-poisoned]
anjali411
added a commit
that referenced
this pull request
Oct 28, 2020
ghstack-source-id: e57f2011fed8add87a0f2ba818f7cbf1d790390f Pull Request resolved: #47024
💊 CI failures summary and remediationsAs of commit cf7edfa (more details on the Dr. CI page):
🕵️ 1 new failure recognized by patternsThe following CI failures do not appear to be due to upstream breakages:
|
albanD
reviewed
Oct 29, 2020
zou3519
reviewed
Oct 29, 2020
zou3519
reviewed
Oct 29, 2020
zou3519
reviewed
Oct 29, 2020
zou3519
reviewed
Oct 29, 2020
Fixes #46936 Stack from [ghstack](https://github.com/ezyang/ghstack): * **#47024 Fix auto exponent issue for torch.pow** [ghstack-poisoned]
Fixes #46936 Stack from [ghstack](https://github.com/ezyang/ghstack): * **#47024 Fix auto exponent issue for torch.pow** [ghstack-poisoned]
anjali411
added a commit
that referenced
this pull request
Oct 29, 2020
ghstack-source-id: e664eaa9159db964a980f8386d532f9dbf7c15df Pull Request resolved: #47024
Fixes #46936 Stack from [ghstack](https://github.com/ezyang/ghstack): * **#47024 Fix auto exponent issue for torch.pow** [ghstack-poisoned]
anjali411
added a commit
that referenced
this pull request
Oct 29, 2020
ghstack-source-id: 3cfe280c6cdeff1cb7984ab79fb6e415a197e2df Pull Request resolved: #47024
anjali411
commented
Oct 29, 2020
| @@ -88,6 +88,17 @@ class C10_API Scalar { | |||
|
|
|||
| Scalar operator-() const; | |||
|
|
|||
| template<typename T> | |||
| bool equal(T num) const { | |||
There was a problem hiding this comment.
cc. @ezyang to verify if it's ok to add equal for Scalar.
Fixes #46936 Stack from [ghstack](https://github.com/ezyang/ghstack): * **#47024 Fix auto exponent issue for torch.pow** [ghstack-poisoned]
anjali411
added a commit
that referenced
this pull request
Oct 30, 2020
ghstack-source-id: 62abe07430dfca7524ff6ca6e47e4ce899714c68 Pull Request resolved: #47024
ezyang
reviewed
Oct 30, 2020
| auto grad_lambda = [](Tensor a, Scalar b) { | ||
| return AT_DISPATCH_DOUBLE_COMPLEXDOUBLE(b.type(), "scalar_val", ([&] { | ||
| scalar_t val = b.to<scalar_t>(); | ||
| return (a * std::log(val)).conj(); |
There was a problem hiding this comment.
This seems a little goofy. Why not also add a log() method on Scalar?
There was a problem hiding this comment.
Should Scalar have more operations defined on it? For example, in pow_backward, we would need to define an operator- for Scalar to avoid the dispatch
There was a problem hiding this comment.
@ezyang added log.
I think it makes sense to support more math operations for Scalar. For now, I added log, and equal in Scalar.h but we should add a new file ScalarMath.{h,cpp} if we plan to add more ops for scalar
Fixes #46936 Stack from [ghstack](https://github.com/ezyang/ghstack): * **#47024 Fix auto exponent issue for torch.pow** [ghstack-poisoned]
anjali411
added a commit
that referenced
this pull request
Nov 3, 2020
ghstack-source-id: 0300183c1941c0287ba4334524c8b79a4df20a95 Pull Request resolved: #47024
zou3519
reviewed
Nov 3, 2020
| return at::zeros_like(self, LEGACY_CONTIGUOUS_MEMORY_FORMAT); | ||
| } else { | ||
| auto out = grad * (exponent * self.pow(exponent - 1)).conj(); | ||
| auto grad_lambda = [&](auto exp) { return grad * (exp * self.pow(exp - 1)).conj(); }; |
There was a problem hiding this comment.
Time to read up on auto type deduction...
There was a problem hiding this comment.
According to https://en.cppreference.com/w/cpp/language/lambda, grad_lambda is a generic lambda, which behaves like a template with one parameter. So this should work.
It's too bad that we can't actually write a test for this
zou3519
approved these changes
Nov 3, 2020
Fixes #46936 Differential Revision: [D24698027](https://our.internmc.facebook.com/intern/diff/D24698027) [ghstack-poisoned]
anjali411
added a commit
that referenced
this pull request
Nov 4, 2020
ghstack-source-id: 6a7db7a10a0a0debf9762892a6b14ee70f85982d Pull Request resolved: #47024
Fixes #46936 Differential Revision: [D24698027](https://our.internmc.facebook.com/intern/diff/D24698027) [ghstack-poisoned]
anjali411
added a commit
that referenced
this pull request
Nov 11, 2020
ghstack-source-id: 5f99cd686fd30520ce16d8e5d1b6599169402dd4 Pull Request resolved: #47024
Fixes #46936 Differential Revision: [D24698027](https://our.internmc.facebook.com/intern/diff/D24698027) [ghstack-poisoned]
anjali411
requested review from
ebetica,
glaringlee,
goldsborough and
yf225
as code owners
anjali411
added a commit
that referenced
this pull request
Nov 13, 2020
ghstack-source-id: f6df8f5fc1f841490eef810e0e413524dc2f1824 Pull Request resolved: #47024
albanD
reviewed
Nov 13, 2020
test/cpp/api/autograd.cpp
Outdated
| // auto y = x.pow(1.5); | ||
| // auto gr = | ||
| // grad({y}, {x}, {}, /*retain_graph=*/true, /*create_backward=*/true); | ||
| // ASSERT_THROWS_WITH(grad({gr[0]}, {x});, "returned nan"); |
There was a problem hiding this comment.
Can you try adding the grad_output as torch::tensor({0.0}) here? That should make it nan as you expect.
Fixes #46936 Differential Revision: [D24698027](https://our.internmc.facebook.com/intern/diff/D24698027) [ghstack-poisoned]
anjali411
added a commit
that referenced
this pull request
Nov 15, 2020
ghstack-source-id: b68bb0477d6bf582852136b497d012ec001dc583 Pull Request resolved: #47024
|
@anjali411 merged this pull request in 8ef7ccd. |
|
Unlanding. This appears to have broken pytorch_linux_xenial_py3_clang5_asan_test2. Relevant snippet: |
|
This pull request has been reverted by 013e6a3. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #46936
Stack from ghstack:
Differential Revision: D24698027