-
Notifications
You must be signed in to change notification settings - Fork 34
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
Support plan_inv
for ScaledPlan's
#77
Conversation
# functionally identical plans | ||
for P in [plan_rfft(x, dims), inv(plan_irfft(ry, size(x, dims), dims)), | ||
AbstractFFTs.plan_inv(plan_irfft(ry, size(x, dims), dims))] | ||
@test eltype(P) <: Real |
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.
Other than add the loop over P
, the only other change I made was to this assertion. Previously the assertion was that the eltype
should be Int
here. That doesn't seem right (for FFTW
the eltype
would be Float64
); rather than thinking too hard about what the test plan implementation should be doing (it's probably underspecified), I just made the check a little looser.
Codecov Report
@@ Coverage Diff @@
## master #77 +/- ##
==========================================
+ Coverage 83.09% 84.13% +1.04%
==========================================
Files 2 2
Lines 207 208 +1
==========================================
+ Hits 172 175 +3
+ Misses 35 33 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
To clarify what's happening here, it's a one-line fix for the downstream issue. I added a few more tests to cover |
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.
Looks good to me.
Fixes a downstream issue in FFTW's tests caused by #72