[FakeTensor] Workaround FFT ops with incorrect meta strides #106319
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This registers a custom fake tensor implemantion for the core fft ops which takes the strides from eager where possible, or otherwise uses unbacked symints to represent the strides. [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/106319
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 2bde2b4: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
peterbell10
added a commit
that referenced
this pull request
Jul 31, 2023
This registers a custom fake tensor implemantion for the core fft ops which takes the strides from eager where possible, or otherwise uses unbacked symints to represent the strides. ghstack-source-id: 0bf9c52c04d54284ce4074c6b98245c38f94e507 Pull Request resolved: #106319
peterbell10
added a commit
to peterbell10/pytorch
that referenced
this pull request
Jul 31, 2023
This registers a custom fake tensor implemantion for the core fft ops which takes the strides from eager where possible, or otherwise uses unbacked symints to represent the strides. ghstack-source-id: 0bf9c52c04d54284ce4074c6b98245c38f94e507 Pull Request resolved: pytorch#106319
…trides" This registers a custom fake tensor implemantion for the core fft ops which takes the strides from eager where possible, or otherwise uses unbacked symints to represent the strides. [ghstack-poisoned]
peterbell10
added a commit
that referenced
this pull request
Jul 31, 2023
This registers a custom fake tensor implemantion for the core fft ops which takes the strides from eager where possible, or otherwise uses unbacked symints to represent the strides. ghstack-source-id: c36c1e121f44133b6ba8c68d72e8efeb6ccae716 Pull Request resolved: #106319
peterbell10
added a commit
to peterbell10/pytorch
that referenced
this pull request
Jul 31, 2023
This registers a custom fake tensor implemantion for the core fft ops which takes the strides from eager where possible, or otherwise uses unbacked symints to represent the strides. ghstack-source-id: c36c1e121f44133b6ba8c68d72e8efeb6ccae716 Pull Request resolved: pytorch#106319
…trides" This registers a custom fake tensor implemantion for the core fft ops which takes the strides from eager where possible, or otherwise uses unbacked symints to represent the strides. [ghstack-poisoned]
peterbell10
added a commit
that referenced
this pull request
Jul 31, 2023
FFT ops meta implementations don't have the correct strides, so currently FakeTensor just raises we need to fall back to eager. ghstack-source-id: 0af17f168826fc8611610a815f97935aa20065b7 Pull Request resolved: #106319
…trides" This registers a custom fake tensor implemantion for the core fft ops which takes the strides from eager where possible, or otherwise uses unbacked symints to represent the strides. [ghstack-poisoned]
peterbell10
added a commit
that referenced
this pull request
Aug 1, 2023
FFT ops meta implementations don't have the correct strides, so currently FakeTensor just raises we need to fall back to eager. ghstack-source-id: 6e031cb1ead241fa69eb6ee4165867a32b8bf557 Pull Request resolved: #106319
…trides" This registers a custom fake tensor implemantion for the core fft ops which takes the strides from eager where possible, or otherwise uses unbacked symints to represent the strides. [ghstack-poisoned]
peterbell10
added a commit
that referenced
this pull request
Aug 1, 2023
FFT ops meta implementations don't have the correct strides, so currently FakeTensor just raises we need to fall back to eager. ghstack-source-id: bdc1f8d5db42b4c5ddaef34aaf75acf27db6bc15 Pull Request resolved: #106319
…trides" This registers a custom fake tensor implemantion for the core fft ops which takes the strides from eager where possible, or otherwise uses unbacked symints to represent the strides. [ghstack-poisoned]
peterbell10
added a commit
to peterbell10/pytorch
that referenced
this pull request
Aug 2, 2023
FFT ops meta implementations don't have the correct strides, so currently FakeTensor just raises we need to fall back to eager. ghstack-source-id: 5c464cf2c06cee99abea9db060dca1534ebd83be Pull Request resolved: pytorch#106319
…trides" This registers a custom fake tensor implemantion for the core fft ops which takes the strides from eager where possible, or otherwise uses unbacked symints to represent the strides. [ghstack-poisoned]
…trides" This registers a custom fake tensor implemantion for the core fft ops which takes the strides from eager where possible, or otherwise uses unbacked symints to represent the strides. [ghstack-poisoned]
…ides" This registers a custom fake tensor implemantion for the core fft ops which takes the strides from eager where possible, or otherwise uses unbacked symints to represent the strides. [ghstack-poisoned]
peterbell10
changed the title
ezyang
approved these changes
Aug 4, 2023
eellison
reviewed
Aug 4, 2023
There was a problem hiding this comment.
I thought these were fixed here. #103616 Or at least, for the opinfos the fft ops now pass stride checks. cc @nkaretnikov
Is this still necessary ?
|
@eellison that PR only touches complex-to-complex transforms, with real-to-complex and complex-to-real still being incorrect. That includes calling Regarding #103616, this PR doesn't apply to Another option that would trade of some runtime performance would be to have inductor use the out variant for these ops. That does cost an extra copy, but at least it would never fail the stride check. |
|
We do have existing precedent for device-specific logic in meta kernels, e.g. here. It doesn't need to be in fake tensor. Ultimately we should fix and add the registrations in meta_registrations. I know it would be a pain to replicate but perhaps a little bit easier with the aid of chat-gpt. Filed #106622 for not having sufficient opinfos to exercise the four dim case. cc @nkaretnikov |
|
Also, I would have expected this pr to fix https://github.com/pytorch/pytorch/blob/main/test/inductor/test_torchinductor_opinfo.py#L240-L257... |
Hrm you're right, those tests pass locally whether I put the xfail there or not. Looks like xpass is broken. |
peterbell10
added a commit
to peterbell10/pytorch
that referenced
this pull request
Aug 4, 2023
FFT ops meta implementations don't have the correct strides, so currently FakeTensor just raises we need to fall back to eager. ghstack-source-id: 9056fd766cb7ff6b6243d38589eb0fc937d0c203 Pull Request resolved: pytorch#106319
This was referenced Aug 5, 2023
peterbell10
added a commit
to peterbell10/pytorch
that referenced
this pull request
Aug 7, 2023
FFT ops meta implementations don't have the correct strides, so currently FakeTensor just raises we need to fall back to eager. ghstack-source-id: 9056fd766cb7ff6b6243d38589eb0fc937d0c203 Pull Request resolved: pytorch#106319
Currently there are FFT operators which raise `UnsupportedOperatorException` because their meta implementations sometimes give incorrect strides. This works around the problem for static shapes by falling back to eager. Though we still don't support calls with dynamic shapes. [ghstack-poisoned]
peterbell10
added a commit
to peterbell10/pytorch
that referenced
this pull request
Aug 7, 2023
FFT ops meta implementations don't have the correct strides, so currently FakeTensor just raises we need to fall back to eager. ghstack-source-id: df7a43236c27ccf5b7ebeaf1248d8dd03b48e58d Pull Request resolved: pytorch#106319
pytorchmergebot
pushed a commit
that referenced
this pull request
Aug 7, 2023
This allows symbolic shapes to be traced through `torch.stft` and `torch.istft`. Pull Request resolved: #106400 Approved by: https://github.com/lezcano ghstack dependencies: #106319
pytorchmergebot
pushed a commit
that referenced
this pull request
Aug 7, 2023
This test raises an error inside the test when an xfailed test succeeds, but is also decorated with the xfail decorator which converts the error to an xfail. Instead, this lets the test function pass normally and lets the xfail decorator raise "Unexpected success". I also updated the COLLECT_EXPECT code and run it to get the updated set of failures. Pull Request resolved: #106631 Approved by: https://github.com/lezcano ghstack dependencies: #106319, #106400
Cyril-Anto
pushed a commit
to Cyril-Anto/pytorch
that referenced
this pull request
Aug 17, 2023
…106319) Currently there are FFT operators which raise `UnsupportedOperatorException` because their meta implementations sometimes give incorrect strides. This works around the problem for static shapes by falling back to eager. Though we still don't support calls with dynamic shapes. Pull Request resolved: pytorch#106319 Approved by: https://github.com/ezyang
Cyril-Anto
pushed a commit
to Cyril-Anto/pytorch
that referenced
this pull request
Aug 17, 2023
…6400) This allows symbolic shapes to be traced through `torch.stft` and `torch.istft`. Pull Request resolved: pytorch#106400 Approved by: https://github.com/lezcano ghstack dependencies: pytorch#106319
Cyril-Anto
pushed a commit
to Cyril-Anto/pytorch
that referenced
this pull request
Aug 17, 2023
This test raises an error inside the test when an xfailed test succeeds, but is also decorated with the xfail decorator which converts the error to an xfail. Instead, this lets the test function pass normally and lets the xfail decorator raise "Unexpected success". I also updated the COLLECT_EXPECT code and run it to get the updated set of failures. Pull Request resolved: pytorch#106631 Approved by: https://github.com/lezcano ghstack dependencies: pytorch#106319, pytorch#106400
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stack from ghstack (oldest at bottom):
Currently there are FFT operators which raise
UnsupportedOperatorExceptionbecause their meta implementations sometimes give incorrect strides. This works
around the problem for static shapes by falling back to eager. Though we still
don't support calls with dynamic shapes.