-
Notifications
You must be signed in to change notification settings - Fork 21.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Inverse Short Time Fourier Transform in ATen native #35569
Conversation
💊 Build failures summary and remediationsAs of commit de4d2e9 (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. This comment has been revised 101 times. |
fed4286
to
3016752
Compare
bb7c8b3
to
0c444a1
Compare
FYI: There is a ongoing discussion on ways to extend |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, LGTM :) see comments below
- Batch packing/unpacking is performed in Python. ATen implementation expects 4D input tensor.
I'm assuming you did so to mirror the stft
implementation? This means that the libtorch implementation will behave differently from the python implementation, right?
- The way
hop_length
is initialized in the same way asstft
implementation. The Torchaudio's version tried to mimic the same behavior but slightly different.
The initialization in ATen seems fine to me. Can you clarify if it changes something for the user in python?
Let's leave these changes for a follow-up PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the tests are green!
LGTM :)
@mthrok -- can you import the PR so that the internal tests run too? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mthrok has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
ISTFT got into PyTorch core. See pytorch/pytorch#35569
Ported
torchaudio
's implementation (test, and documentation as well) to ATen.Note
hop_length
is initialized in the same way asstft
implementation. The Torchaudio's version tried to mimic the same behavior but slightly different.Closes #34827
Relates #3775