Skip to content

Add Inverse Short Time Fourier Transform in ATen native#35569

Closed
mthrok wants to merge 8 commits intopytorch:masterfrom
mthrok:aten-isftf
Closed

Add Inverse Short Time Fourier Transform in ATen native#35569
mthrok wants to merge 8 commits intopytorch:masterfrom
mthrok:aten-isftf

Conversation

@mthrok
Copy link
Copy Markdown
Contributor

@mthrok mthrok commented Mar 27, 2020 •
edited
Loading

Ported torchaudio's implementation (test, and documentation as well) to ATen.

Note

Closes #34827
Relates #3775

@mthrok mthrok requested a review from apaszke as a code owner March 27, 2020 20:44
@mthrok mthrok requested a review from vincentqb March 27, 2020 20:45
@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Mar 27, 2020
@dr-ci
Copy link
Copy Markdown

dr-ci Bot commented Mar 27, 2020 •
edited
Loading

💊 Build failures summary and remediations

As 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.

See how this bot performed.

This comment has been revised 101 times.

@mthrok mthrok force-pushed the aten-isftf branch 5 times, most recently from fed4286 to 3016752 Compare April 1, 2020 18:59
@mthrok mthrok changed the title Add Inverse Short Time Fourier Transform Add Inverse Short Time Fourier Transform in ATen native Apr 1, 2020
@mthrok mthrok force-pushed the aten-isftf branch 8 times, most recently from bb7c8b3 to 0c444a1 Compare April 7, 2020 14:14
@mthrok mthrok mentioned this pull request Apr 7, 2020
Merged
@mthrok
Copy link
Copy Markdown
Contributor Author

mthrok commented Apr 8, 2020

FYI: There is a ongoing discussion on ways to extend istft for custom padding pytorch/audio#500 pytorch/audio#427 .

vincentqb
vincentqb suggested changes Apr 8, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@vincentqb vincentqb left a comment •
edited
Loading

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 initialization in ATen seems fine to me. Can you clarify if it changes something for the user in python?

Comment thread aten/src/ATen/native/SpectralOps.cpp Outdated
Comment thread torch/_tensor_docs.py Outdated
Comment thread torch/_tensor_docs.py Outdated
Comment thread torch/functional.py Outdated
@vincentqb
Copy link
Copy Markdown
Contributor

vincentqb commented Apr 8, 2020

FYI: There is a ongoing discussion on ways to extend istft for custom padding pytorch/audio#500 pytorch/audio#427 .

Let's leave these changes for a follow-up PR.

vincentqb
vincentqb reviewed Apr 9, 2020
View reviewed changes
Comment thread test/test_torch.py Outdated
@mthrok mthrok removed the request for review from apaszke April 20, 2020 18:40
vincentqb
vincentqb approved these changes Apr 21, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@vincentqb vincentqb left a 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 :)

@vincentqb
Copy link
Copy Markdown
Contributor

vincentqb commented Apr 21, 2020 •
edited
Loading

@mthrok -- can you import the PR so that the internal tests run too?

facebook-github-bot
facebook-github-bot reviewed Apr 22, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a 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.

@mthrok mthrok deleted the aten-isftf branch April 24, 2020 19:27
@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Apr 24, 2020

@mthrok merged this pull request in 5a27ec0.

seungwonpark added a commit to seungwonpark/istft-pytorch that referenced this pull request May 19, 2020
@seungwonpark
update README regarding torch.istft
aa4eda6 
ISTFT got into PyTorch core. See pytorch/pytorch#35569
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
@mthrok @facebook-github-bot
Add Inverse Short Time Fourier Transform in ATen native (pytorch#35569)
492413e 
Summary:
Ported `torchaudio`'s implementation (test, and documentation as well) to ATen.

Note
 - Batch packing/unpacking is performed in Python. ATen implementation expects 4D input tensor.
 - The way `hop_length` is initialized in the same way as `stft` implementation. [The Torchaudio's version tried to mimic the same behavior but slightly different](https://github.com/pytorch/audio/blob/7da61a4beeec7b7ff9ff5f1532b2adf99220a9b1/torchaudio/functional.py#L152-L157).

Closes pytorch#34827
Relates pytorch#3775
Pull Request resolved: pytorch#35569

Differential Revision: D21178090

Pulled By: mthrok

fbshipit-source-id: 2701a8b241a36a6fb1b740c2fb2b07cb938185d4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@vincentqb vincentqb vincentqb approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Merged oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mthrok @vincentqb @facebook-github-bot @mruberry