Skip to content
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

[ONNX] Opset 17 STFT support #83944

Closed
wants to merge 1 commit into from

Conversation

justinchuby
Copy link
Collaborator

@justinchuby justinchuby commented Aug 23, 2022

Stack from ghstack (oldest at bottom):

#81075

TODO:

  • Split complex input into real
  • Test

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Aug 23, 2022

🔗 Helpful links

❌ 1 New Failures, 1 Base Failures

As of commit f304c53 (more details on the Dr. CI page):

Expand to see more
  • 1/2 failures introduced in this PR
  • 1/2 broken upstream at merge base 7e38684 on Aug 23 from 9:38am to 2:13pm

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages

See GitHub Actions build Lint / lintrunner (1/1)

Step: "Run lintrunner on all files" (full log | diagnosis details)

2022-08-23T22:25:50.6679450Z ##[error]Process completed with exit code 1.
2022-08-23T22:25:50.6666182Z         �[2m24�[0m  |import torch
2022-08-23T22:25:50.6666529Z         �[2m25�[0m  |from torch import _C
2022-08-23T22:25:50.6666847Z     >>> �[2m26�[0m  |�[33mfrom torch._C import _onnx as _C_onnx
2022-08-23T22:25:50.6667211Z �[0m        �[2m27�[0m  |from torch.onnx import _type_utils, symbolic_helper
2022-08-23T22:25:50.6667476Z         �[2m28�[0m  |
2022-08-23T22:25:50.6667691Z         �[2m29�[0m  |
2022-08-23T22:25:50.6667801Z 
2022-08-23T22:25:50.6667807Z 
2022-08-23T22:25:50.6668034Z �[1m�[36mYou can reproduce these results locally by using `lintrunner`.�[0m
2022-08-23T22:25:50.6668489Z �[1m�[36mSee https://github.com/pytorch/pytorch/wiki/lintrunner for setup instructions.�[0m
2022-08-23T22:25:50.6679450Z ##[error]Process completed with exit code 1.
2022-08-23T22:25:50.6714581Z ##[group]Run # Use jq to massage the JSON lint output into GitHub Actions workflow commands.
2022-08-23T22:25:50.6715042Z �[36;1m# Use jq to massage the JSON lint output into GitHub Actions workflow commands.�[0m
2022-08-23T22:25:50.6715381Z �[36;1mjq --raw-output \�[0m
2022-08-23T22:25:50.6715885Z �[36;1m  '"::\(if .severity == "advice" or .severity == "disabled" then "warning" else .severity end) file=\(.path),line=\(.line),col=\(.char),title=\(.code) \(.name)::" + (.description | gsub("\\n"; "%0A"))' \�[0m
2022-08-23T22:25:50.6716265Z �[36;1m  lint.json�[0m
2022-08-23T22:25:50.6762627Z shell: /usr/bin/bash -e {0}
2022-08-23T22:25:50.6762958Z env:
2022-08-23T22:25:50.6763234Z   pythonLocation: /opt/hostedtoolcache/Python/3.8.13/x64
2022-08-23T22:25:50.6763601Z   LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.8.13/x64/lib
2022-08-23T22:25:50.6763897Z ##[endgroup]

🚧 1 fixed upstream failure:

These were probably caused by upstream breakages that were already fixed.

Please rebase on the viable/strict branch (expand for instructions)

If your commit is older than viable/strict, run these commands:

git fetch https://github.com/pytorch/pytorch viable/strict
git rebase FETCH_HEAD

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

justinchuby added a commit that referenced this pull request Aug 23, 2022
ghstack-source-id: be5336e569ba3577994bc0f323a7502cc7c6b2d0
Pull Request resolved: #83944
@justinchuby justinchuby marked this pull request as draft August 23, 2022 22:18
@facebook-github-bot
Copy link
Contributor

/easycla

As part of the transition to the PyTorch Foundation, this project now requires contributions be covered under the new CLA. See #85559 for additional details.

This comment will trigger a new check of this PR. If you are already covered, you will simply see a new "EasyCLA" check that passes. If you are not covered, a bot will leave a new comment with a link to sign.

@linux-foundation-easycla
Copy link

CLA Not Signed

@nateanl
Copy link
Member

nateanl commented Oct 7, 2022

Hi @justinchuby, thanks for working on the STFT support. Is the PR ready for review?

@justinchuby
Copy link
Collaborator Author

@nateanl thanks for asking! The PR is not ready yet. I started it and got distracted.

@mravanelli
Copy link

Thank you @justinchuby. Is there any news? It would be really great to have in in the next version of pytorch. This is something we actually need for SpeechBrain because we need the feature extraction pipeline to be ONN-exportable.

@justinchuby
Copy link
Collaborator Author

@mravanelli A more realistic issue I realized is ONNX doesn't have the appropriate ops to handle complex <-> real matrix conversion. Since the STFT op in python operates on complex numbers and onnx STFT operates on real numbers, I will need to think harder about how this is possible.

Assuming onnx does support the operator, you can always implement and register a custom symbolic function (https://pytorch.org/docs/stable/onnx.html#adding-support-for-operators) and not have to worry about pytorch release cycles.

justinchuby added a commit to justinchuby/pytorch that referenced this pull request Oct 11, 2022
ghstack-source-id: be5336e569ba3577994bc0f323a7502cc7c6b2d0
Pull Request resolved: pytorch#83944
@github-actions
Copy link

github-actions bot commented Dec 9, 2022

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

@github-actions github-actions bot added the Stale label Dec 9, 2022
@github-actions github-actions bot closed this Jan 9, 2023
@urinieto urinieto mentioned this pull request Jan 12, 2023
pytorchmergebot pushed a commit that referenced this pull request Mar 10, 2023
This PR addresses issue [#81075](#81075),  making `torch.stft` compatible with ONNX Opset 17's STFT operator.

The conversion works for _most_ of `torch.stft` functionality:

- Batched or unbatched inputs
- Normalization
- Pre-computed windows
- Rectangular windows
- One-sided returns
- Window centering (implicitly supported)

What is currently _not_ supported is **complex types**, due to the lack of conversion functionality between PyTorch and ONNX (#86746).

Regardless, this is easy to bypass by setting `return_complex=False` when using `torch.stft`.

Note that there is already a draft PR to address this (#83944), but it is currently closed and it only partially addresses the conversion (i.e., most of `torch.stft` functionality is lacking, and unit tests are missing).
Pull Request resolved: #92087
Approved by: https://github.com/justinchuby
cyyever pushed a commit to cyyever/pytorch_private that referenced this pull request Mar 12, 2023
This PR addresses issue [#81075](pytorch/pytorch#81075),  making `torch.stft` compatible with ONNX Opset 17's STFT operator.

The conversion works for _most_ of `torch.stft` functionality:

- Batched or unbatched inputs
- Normalization
- Pre-computed windows
- Rectangular windows
- One-sided returns
- Window centering (implicitly supported)

What is currently _not_ supported is **complex types**, due to the lack of conversion functionality between PyTorch and ONNX (pytorch/pytorch#86746).

Regardless, this is easy to bypass by setting `return_complex=False` when using `torch.stft`.

Note that there is already a draft PR to address this (pytorch/pytorch#83944), but it is currently closed and it only partially addresses the conversion (i.e., most of `torch.stft` functionality is lacking, and unit tests are missing).
Pull Request resolved: pytorch/pytorch#92087
Approved by: https://github.com/justinchuby
@facebook-github-bot facebook-github-bot deleted the gh/justinchuby/6/head branch June 8, 2023 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants