-
Notifications
You must be signed in to change notification settings - Fork 99
[torchlib] Fix irfft #2770
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
[torchlib] Fix irfft #2770
Conversation
6baaef6 to
b9462d2
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2770 +/- ##
=======================================
Coverage 70.45% 70.45%
=======================================
Files 228 228
Lines 27257 27258 +1
Branches 2760 2761 +1
=======================================
+ Hits 19203 19204 +1
Misses 7102 7102
Partials 952 952 ☔ View full report in Codecov by Sentry. |
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.
lintrunner found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Signed-off-by: Simon Byrne <sbyrne@nvidia.com>
b9462d2 to
7b26868
Compare
Signed-off-by: Simon Byrne <sbyrne@nvidia.com>
9ac8d5a to
76756e6
Compare
justinchuby
left a comment
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.
Thank you!
|
Unfortunately I think the tests won't pass until onnx/onnx#7574 and microsoft/onnxruntime#27028 are both merged. |
|
Could you make a special case for len(dim)=1, such that existing tests can pass? Or, do you think the original implementation were wrong (did not account for the neg frequencies)? |
|
If you think the original impl was wrong. Please feel free to skip the tests |
Signed-off-by: Simon Byrne <sbyrne@nvidia.com>
|
No, the existing implementation is definitely wrong. I added an xfail until onnx and onnxruntime can be updated. |
This fixes the onnxscript export for the irfft function.
Fixes onnx/onnx#5920, and adds support to the changes in onnx/onnx#7574 and microsoft/onnxruntime#27028.
Most of the diff is due to the onnx_opset generated code changes from onnx/onnx#5920. That can be removed if you would prefer.