stft: Require return_complex to be passed explicitly for real input #86724
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
This behavior has been deprecated since PyTorch 1.8 but this step of the deprecation cycle was put on hold in #50102 waiting for JIT upgraders functionality which doesn't seem to have panned out. I'd say there has been more than enough of a deprecation period, so we should just continue. BC-breaking message: `torch.stft` takes an optional `return_complex` parameter that indicates whether the output should be a floating point tensor or a complex tensor. `return_complex` previously defaulted to `False` for real input tensors. This PR removes the default and makes `return_complex` a required argument for real inputs. However, complex inputs will continue to default to `return_complex=True`. [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/86724
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 792aa85: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
peterbell10
added
module: bc-breaking
mruberry
reviewed
Oct 14, 2022
| y = x.stft(10, pad_mode='constant') | ||
| # with self.assertRaisesRegex(RuntimeError, 'stft requires the return_complex parameter'): | ||
| # y = x.stft(10, pad_mode='constant') | ||
| with self.assertRaisesRegex(RuntimeError, 'stft requires the return_complex parameter'): |
Collaborator
There was a problem hiding this comment.
Nit: this could be an error input
mruberry
reviewed
Oct 14, 2022
| a real tensor with an extra last dimension for the real and | ||
| imaginary components. | ||
| .. versionchanged:: 1.14.0 |
mruberry
reviewed
Oct 14, 2022
torch/functional.py
Outdated
| .. versionchanged:: 1.14.0 | ||
| ``return_complex`` is now a required argument for real inputs, | ||
| as the default is being trainsitioned to ``True``. |
Collaborator
There was a problem hiding this comment.
"trainsitioned" -> "transitioned"
mruberry
approved these changes
Oct 14, 2022
Collaborator
mruberry
left a comment
mruberry
left a comment
There was a problem hiding this comment.
This seems reasonable to me -- @ngimel @albanD is there someone at Meta who should sanity-check the BC implications?
@peterbell10 take a look at my questions about the docs, however -- I think they can be streamlined
albanD
approved these changes
Oct 14, 2022
Collaborator
albanD
left a comment
albanD
left a comment
There was a problem hiding this comment.
I think we should just land this!
Checking is so painful that it is easier to try and land it :)
If it comes back to us we can go down the rabbit hole.
…eal input" This behavior has been deprecated since PyTorch 1.8 but this step of the deprecation cycle was put on hold in #50102 waiting for JIT upgraders functionality which doesn't seem to have panned out. I'd say there has been more than enough of a deprecation period, so we should just continue. BC-breaking message: `torch.stft` takes an optional `return_complex` parameter that indicates whether the output should be a floating point tensor or a complex tensor. `return_complex` previously defaulted to `False` for real input tensors. This PR removes the default and makes `return_complex` a required argument for real inputs. However, complex inputs will continue to default to `return_complex=True`. [ghstack-poisoned]
albanD
approved these changes
Oct 16, 2022
…eal input" This behavior has been deprecated since PyTorch 1.8 but this step of the deprecation cycle was put on hold in #50102 waiting for JIT upgraders functionality which doesn't seem to have panned out. I'd say there has been more than enough of a deprecation period, so we should just continue. BC-breaking message: `torch.stft` takes an optional `return_complex` parameter that indicates whether the output should be a floating point tensor or a complex tensor. `return_complex` previously defaulted to `False` for real input tensors. This PR removes the default and makes `return_complex` a required argument for real inputs. However, complex inputs will continue to default to `return_complex=True`. [ghstack-poisoned]
Collaborator
Author
|
@pytorchbot merge |
Collaborator
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
peterbell10
added a commit
to peterbell10/pytorch
that referenced
this pull request
Oct 17, 2022
This behavior has been deprecated since PyTorch 1.8 but this step of the deprecation cycle was put on hold in pytorch#50102 waiting for JIT upgraders functionality which doesn't seem to have panned out. I'd say there has been more than enough of a deprecation period, so we should just continue. BC-breaking message: `torch.stft` takes an optional `return_complex` parameter that indicates whether the output should be a floating point tensor or a complex tensor. `return_complex` previously defaulted to `False` for real input tensors. This PR removes the default and makes `return_complex` a required argument for real inputs. However, complex inputs will continue to default to `return_complex=True`. ghstack-source-id: fba3d91 Pull Request resolved: pytorch#86724
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 (oldest at bottom):
This behavior has been deprecated since PyTorch 1.8 but this step of
the deprecation cycle was put on hold in #50102 waiting for JIT
upgraders functionality which doesn't seem to have panned out. I'd say
there has been more than enough of a deprecation period, so we should
just continue.
BC-breaking message:
torch.stfttakes an optionalreturn_complexparameter thatindicates whether the output should be a floating point tensor or a
complex tensor.
return_complexpreviously defaulted toFalseforreal input tensors. This PR removes the default and makes
return_complexa required argument for real inputs. However, complexinputs will continue to default to
return_complex=True.1.13
2.0