Remove deprecated spectral ops from torch namespace#48594
Remove deprecated spectral ops from torch namespace#48594peterbell10 wants to merge 2 commits intopytorch:masterfrom
Conversation
💊 CI failures summary and remediationsAs of commit 99ff080 (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 12 times. |
|
Hey @peterbell10! This is great. Just a couple more places to cleanup: overrides docs has issues like this: back compat needs an update, too: back compat needs entries here: |
There was a problem hiding this comment.
Does fft.h need updates to prevent calling these functions from the C++ API, too?
There was a problem hiding this comment.
fft.h only deals with the fft module, the torch::fft functions was just generated by codegen so is gone completely. And actually as you point to this include, it's not needed any more since torch.h automatically includes the fft namespace as well now.
0da6321 to
7052251
Compare
7052251 to
99ff080
Compare
| "The function torch.fft is deprecated and will be removed in PyTorch 1.8. " | ||
| "Use the new torch.fft module functions, instead, by importing torch.fft " | ||
| "and calling torch.fft.fft or torch.fft.fftn."); | ||
| static Tensor fft(const Tensor& self, const int64_t signal_ndim, const bool normalized) { |
There was a problem hiding this comment.
One question here. Why we need static? Is that because we want internal linkage for these functions so they are visible in this file only? (fft, ifft, rfft, irfft).
There was a problem hiding this comment.
Yes, the only remaining users (stft and istft) are in the same file. Marked static so we know for sure it isn't in the public API, or even used elsewhere internally.
Codecov Report
@@ Coverage Diff @@
## master #48594 +/- ##
==========================================
- Coverage 80.91% 80.90% -0.02%
==========================================
Files 1855 1855
Lines 200241 200225 -16
==========================================
- Hits 162023 161988 -35
- Misses 38218 38237 +19 |
|
Remember to ping when this is ready for another review ;) |
|
Oh sorry, @mruberry PTAL. |
mruberry
left a comment
There was a problem hiding this comment.
Awesome! Thanks @peterbell10!
facebook-github-bot
left a comment
There was a problem hiding this comment.
@mruberry has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
FYI @peterbell10 this won't land immediately because some libraries will need time to update their deprecated code. |
|
@Balandat's cornellius-gp/linear_operator#11 unblocked this. Thanks again, @Balandat! |
Summary: Ref pytorch#42175 This removes the 4 deprecated spectral functions: `torch.{fft,rfft,ifft,irfft}`. `torch.fft` is also now imported by by default. The actual `at::native` functions are still used in `torch.stft` so can't be full removed yet. But will once pytorch#47601 has been merged. Pull Request resolved: pytorch#48594 Reviewed By: heitorschueroff Differential Revision: D25298929 Pulled By: mruberry fbshipit-source-id: e36737fe8192fcd16f7e6310f8b49de478e63bf0
BC-breaking change:
This PR, as the PR summary states, removes the deprecated
torch.{fft,rfft,ifft,irfft}and their corresponding methods on torch.Tensor. PyTorch programs using these functions must now update to use the torch.fft namespace.Original PR Summary:
Ref #42175
This removes the 4 deprecated spectral functions:
torch.{fft,rfft,ifft,irfft}.torch.fftis also now imported by by default.The actual
at::nativefunctions are still used intorch.stftso can't be full removed yet. But will once #47601 has been merged.