torch.sgn for complex tensors #39955
Closed
Conversation
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
[ghstack-poisoned]
Contributor
Author
|
cc. @dylanbespalko for the Vec256 changes |
💊 CI failures summary and remediationsAs of commit 0fed8c8 (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 on the GitHub issue tracker or post in the (internal) Dr. CI Users group. This comment has been revised 95 times. |
mruberry
reviewed
Jun 12, 2020
mruberry
reviewed
Jun 12, 2020
mruberry
reviewed
Jun 12, 2020
| ['sigmoid', torch.sigmoid], | ||
| ['sigmoid_', torch.sigmoid_], | ||
| ['sign', torch.sign], | ||
| ['sgn', torch.sign], |
mruberry
reviewed
Jun 12, 2020
mruberry
reviewed
Jun 12, 2020
mruberry
reviewed
Jun 12, 2020
torch/_torch_docs.py
Outdated
| Example:: | ||
| >>> x=torch.randn(4, dtype=torch.cfloat) |
Collaborator
There was a problem hiding this comment.
The example doesn't really demonstrate the function because of the random values. Maybe pick some angles, like 0, 45, 90 degrees?
mruberry
reviewed
Jun 12, 2020
mruberry
reviewed
Jun 12, 2020
test/test_torch.py
Outdated
| cos_angle = angle.cos() | ||
| sin_angle = angle.sin() | ||
| expected = cos_angle + 1j * sin_angle | ||
| self.assertEqual(x.sgn(), expected) |
Collaborator
There was a problem hiding this comment.
Can this instead assert that x.sgn has the same angle as x.angle and that the absolute value is everywhere one?
Collaborator
|
Added some notes. This also still needs to update torch/_overrides.py, docs/source/tensors.rst, docs/source/torch.rst, docs/source/name_inference.rst, aten/src/ATen/core/aten_interned_strings.h and tools/derivatives.yaml. Can this also be tested by TestTensorDeviceOps and TestTorchMathOps? |
Contributor
ezyang
reviewed
Jun 15, 2020
[ghstack-poisoned]
ezyang
requested changes
Jun 16, 2020
Contributor
ezyang
left a comment
ezyang
left a comment
There was a problem hiding this comment.
Don't compute normalized vector using trig functions
resolves #36323 by adding `torch.sgn` for complex tensors. `torch.sgn` returns `x/abs(x)` for `x != 0` and returns `0 + 0j` for `x==0` TODO: 1. add tests for backward (waiting on gradcheck PR for complex) [ghstack-poisoned]
mruberry
reviewed
Jul 31, 2020
mruberry
reviewed
Jul 31, 2020
mruberry
approved these changes
Jul 31, 2020
Collaborator
mruberry
left a comment
mruberry
left a comment
There was a problem hiding this comment.
Minor doc nits but otherwise this looks good to me!
resolves #36323 by adding `torch.sgn` for complex tensors. `torch.sgn` returns `x/abs(x)` for `x != 0` and returns `0 + 0j` for `x==0` TODO: 1. add tests for backward (waiting on gradcheck PR for complex) [ghstack-poisoned]
albanD
reviewed
Sep 3, 2020
Collaborator
albanD
left a comment
albanD
left a comment
There was a problem hiding this comment.
I think this still needs the gradients for the real case to be tested, even if you postpone the check for the complex version.
Contributor
Author
I was thinking of waiting to merge this PR until #43208 is merged and add tests for sgn. |
resolves #36323 by adding `torch.sgn` for complex tensors. `torch.sgn` returns `x/abs(x)` for `x != 0` and returns `0 + 0j` for `x==0` This PR doesn't test the correctness of the gradients. It will be done as a part of auditing all the ops in future once we decide the autograd behavior (JAX vs TF) and add gradchek. Differential Revision: [D23460526](https://our.internmc.facebook.com/intern/diff/D23460526) [ghstack-poisoned]
resolves #36323 by adding `torch.sgn` for complex tensors. `torch.sgn` returns `x/abs(x)` for `x != 0` and returns `0 + 0j` for `x==0` This PR doesn't test the correctness of the gradients. It will be done as a part of auditing all the ops in future once we decide the autograd behavior (JAX vs TF) and add gradchek. Differential Revision: [D23460526](https://our.internmc.facebook.com/intern/diff/D23460526) [ghstack-poisoned]
resolves #36323 by adding `torch.sgn` for complex tensors. `torch.sgn` returns `x/abs(x)` for `x != 0` and returns `0 + 0j` for `x==0` also updates the backward definition of `torch.div`, `torch.abs` Differential Revision: [D23460526](https://our.internmc.facebook.com/intern/diff/D23460526) [ghstack-poisoned]
resolves #36323 by adding `torch.sgn` for complex tensors. `torch.sgn` returns `x/abs(x)` for `x != 0` and returns `0 + 0j` for `x==0` also updates the backward definition of `torch.div`, `torch.abs` Differential Revision: [D23460526](https://our.internmc.facebook.com/intern/diff/D23460526) [ghstack-poisoned]
resolves #36323 by adding `torch.sgn` for complex tensors. `torch.sgn` returns `x/abs(x)` for `x != 0` and returns `0 + 0j` for `x==0` also updates the backward definition of `torch.div`, `torch.abs` Differential Revision: [D23460526](https://our.internmc.facebook.com/intern/diff/D23460526) [ghstack-poisoned]
Codecov Report
@@ Coverage Diff @@
## gh/anjali411/34/base #39955 +/- ##
========================================================
- Coverage 67.86% 67.85% -0.01%
========================================================
Files 384 384
Lines 50026 50029 +3
========================================================
Hits 33948 33948
- Misses 16078 16081 +3
Continue to review full report at Codecov.
|
resolves #36323 by adding `torch.sgn` for complex tensors. `torch.sgn` returns `x/abs(x)` for `x != 0` and returns `0 + 0j` for `x==0` also updates the backward definition of `torch.div`, `torch.abs` Differential Revision: [D23460526](https://our.internmc.facebook.com/intern/diff/D23460526) [ghstack-poisoned]
resolves #36323 by adding `torch.sgn` for complex tensors. `torch.sgn` returns `x/abs(x)` for `x != 0` and returns `0 + 0j` for `x==0` also updates the backward definition of `torch.div`, `torch.abs` Differential Revision: [D23460526](https://our.internmc.facebook.com/intern/diff/D23460526) [ghstack-poisoned]
Contributor
|
@anjali411 merged this pull request in 58b6ab6. |
loadbxh
pushed a commit
to loadbxh/Torch
that referenced
this pull request
Sep 23, 2020
ghstack-source-id: e4ab67a Pull Request resolved: pytorch/pytorch#39955
anjali411
added a commit
that referenced
this pull request
Sep 29, 2020
This PR disables autograd for all C -> C, R -> C functions which are not included in the whitelist `GRADIENT_IMPLEMENTED_FOR_COMPLEX`. In practice, there will be a RuntimeError during forward computation when the outputs are differentiable: ``` >>> x=torch.randn(4, 4, requires_grad=True, dtype=torch.cdouble) >>> x.pow(3) Traceback (most recent call last): File "<stdin>", line 1, in <module> RuntimeError: pow does not support automatic differentiation for outputs with complex dtype. ``` The implicit assumption here is that all the C -> R functions have correct backward definitions. So before merging this PR, the following functions must be tested and verified to have correct backward definitions: `torch.abs` (updated in #39955 ), `torch.angle`, `torch.norm`, `torch.irfft`, `torch.istft`. [ghstack-poisoned]
anjali411
added a commit
that referenced
this pull request
Sep 29, 2020
This PR disables autograd for all C -> C, R -> C functions which are not included in the whitelist `GRADIENT_IMPLEMENTED_FOR_COMPLEX`. In practice, there will be a RuntimeError during forward computation when the outputs are differentiable: ``` >>> x=torch.randn(4, 4, requires_grad=True, dtype=torch.cdouble) >>> x.pow(3) Traceback (most recent call last): File "<stdin>", line 1, in <module> RuntimeError: pow does not support automatic differentiation for outputs with complex dtype. ``` The implicit assumption here is that all the C -> R functions have correct backward definitions. So before merging this PR, the following functions must be tested and verified to have correct backward definitions: `torch.abs` (updated in #39955 ), `torch.angle`, `torch.norm`, `torch.irfft`, `torch.istft`. [ghstack-poisoned]
anjali411
added a commit
that referenced
this pull request
Sep 29, 2020
This PR disables autograd for all C -> C, R -> C functions which are not included in the whitelist `GRADIENT_IMPLEMENTED_FOR_COMPLEX`. In practice, there will be a RuntimeError during forward computation when the outputs are differentiable: ``` >>> x=torch.randn(4, 4, requires_grad=True, dtype=torch.cdouble) >>> x.pow(3) Traceback (most recent call last): File "<stdin>", line 1, in <module> RuntimeError: pow does not support automatic differentiation for outputs with complex dtype. ``` The implicit assumption here is that all the C -> R functions have correct backward definitions. So before merging this PR, the following functions must be tested and verified to have correct backward definitions: `torch.abs` (updated in #39955 ), `torch.angle`, `torch.norm`, `torch.irfft`, `torch.istft`. [ghstack-poisoned]
anjali411
added a commit
that referenced
this pull request
Sep 30, 2020
This PR disables autograd for all C -> C, R -> C functions which are not included in the whitelist `GRADIENT_IMPLEMENTED_FOR_COMPLEX`. In practice, there will be a RuntimeError during forward computation when the outputs are differentiable: ``` >>> x=torch.randn(4, 4, requires_grad=True, dtype=torch.cdouble) >>> x.pow(3) Traceback (most recent call last): File "<stdin>", line 1, in <module> RuntimeError: pow does not support automatic differentiation for outputs with complex dtype. ``` The implicit assumption here is that all the C -> R functions have correct backward definitions. So before merging this PR, the following functions must be tested and verified to have correct backward definitions: `torch.abs` (updated in #39955 ), `torch.angle`, `torch.norm`, `torch.irfft`, `torch.istft`. Differential Revision: [D23998156](https://our.internmc.facebook.com/intern/diff/D23998156) [ghstack-poisoned]
anjali411
added a commit
that referenced
this pull request
Sep 30, 2020
This PR disables autograd for all C -> C, R -> C functions which are not included in the whitelist `GRADIENT_IMPLEMENTED_FOR_COMPLEX`. In practice, there will be a RuntimeError during forward computation when the outputs are differentiable: ``` >>> x=torch.randn(4, 4, requires_grad=True, dtype=torch.cdouble) >>> x.pow(3) Traceback (most recent call last): File "<stdin>", line 1, in <module> RuntimeError: pow does not support automatic differentiation for outputs with complex dtype. ``` The implicit assumption here is that all the C -> R functions have correct backward definitions. So before merging this PR, the following functions must be tested and verified to have correct backward definitions: `torch.abs` (updated in #39955 ), `torch.angle`, `torch.norm`, `torch.irfft`, `torch.istft`. Differential Revision: [D23998156](https://our.internmc.facebook.com/intern/diff/D23998156) [ghstack-poisoned]
facebook-github-bot
pushed a commit
that referenced
this pull request
Sep 30, 2020
Summary: Pull Request resolved: #45461 This PR disables autograd for all C -> C, R -> C functions which are not included in the whitelist `GRADIENT_IMPLEMENTED_FOR_COMPLEX`. In practice, there will be a RuntimeError during forward computation when the outputs are differentiable: ``` >>> x=torch.randn(4, 4, requires_grad=True, dtype=torch.cdouble) >>> x.pow(3) Traceback (most recent call last): File "<stdin>", line 1, in <module> RuntimeError: pow does not support automatic differentiation for outputs with complex dtype. ``` The implicit assumption here is that all the C -> R functions have correct backward definitions. So before merging this PR, the following functions must be tested and verified to have correct backward definitions: `torch.abs` (updated in #39955 ), `torch.angle`, `torch.norm`, `torch.irfft`, `torch.istft`. Test Plan: Imported from OSS Reviewed By: malfet Differential Revision: D23998156 Pulled By: anjali411 fbshipit-source-id: 370eb07fe56ac84dd8e2233ef7bf3a3eb8aeb179
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.
Stack from ghstack:
resolves #36323 by adding
torch.sgnfor complex tensors.torch.sgnreturnsx/abs(x)forx != 0and returns0 + 0jforx==0also updates the backward definition of
torch.div,torch.absDifferential Revision: D23460526