Fix inverted nbatch/size check in FFT._check_fwd_args - #5396
Open
ahnitz wants to merge 1 commit into
Open
Conversation
The forward-FFT argument check rejected the only valid way to request a batched transform (nbatch > 1 with an explicit size) while silently accepting the invalid nbatch > 1, size=None case. _check_inv_args already had the correct condition; batched forward FFTs via the class-based API just hadn't been exercised elsewhere in the codebase until now.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The forward-FFT argument check rejected the only valid way to request a batched transform (nbatch > 1 with an explicit size) while silently accepting the invalid nbatch > 1, size=None case. _check_inv_args already had the correct condition; batched forward FFTs via the class-based API just hadn't been exercised elsewhere in the codebase until now.
Standard information about the request
This is a bug fix.
This change affects nothing as we didn't use this option in any codes.
This change: has appropriate unit tests, follows style guidelines (See e.g. PEP8), has been proposed using the contribution guidelines
This change will: break current functionality, require additional dependencies, require a new release, other (please describe)
Motivation
Contents
Links to any issues or associated PRs
Testing performed
I've tested this in a downstream PR #5395.
Additional notes
This is required for #5395