disallow plan * real for in-place complex plan#342
Conversation
When applying a complex inline plan to a real vector, '*' should throw similar to 'mul!'.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #342 +/- ##
==========================================
+ Coverage 73.45% 73.89% +0.43%
==========================================
Files 5 5
Lines 535 544 +9
==========================================
+ Hits 393 402 +9
Misses 142 142 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
s/inline/inplace/ |
stevengj
changed the title
plan * real for in-place complex plan
Co-authored-by: Steven G. Johnson <stevenj@mit.edu>
Co-authored-by: Mosè Giordano <765740+giordano@users.noreply.github.com>
|
|
| end | ||
|
|
||
|
|
||
| function assert_applicable(p::FFTWPlan{T}, X::AbstractArray) where T |
There was a problem hiding this comment.
shouldn't this be restricted to in-place plans?
|
Well, that case was handeled by But I agree that testing for inline in Ah and method dispatch based on the type parameter via defining |
|
|
||
| function assert_applicable(p::FFTWPlan{T}, X::AbstractArray) where T | ||
| throw(ArgumentError("In-place plan FFTWPlan{$(T)} cannot be applied to a $(typeof(X))")) | ||
| function assert_applicable(p::FFTWPlan{T,K, inplace}, X::AbstractArray) where {T,K, inplace} |
There was a problem hiding this comment.
Instead of an if inplace stqtement, why not just make only to p::FFTWPlan{<:Any,<:Any,true}?
There was a problem hiding this comment.
Hmm, we need to make sure that the parent assert_applicable is called when appropriate too…
|
|
||
| function mul!(y::AbstractArray, p::cFFTWPlan{T}, x::AbstractArray) where T | ||
| throw(ArgumentError("mul!(x,::cFFTWPlan{$(T)},y) expects x and y be of type StridedArray{$(T)}")) | ||
| end |
There was a problem hiding this comment.
Why is this method necessary? If you just delete this method won't it throw a MethodError?
3 participants
When applying a complex in-place plan to a real vector, '*' should throw similar to 'mul!'.
However, AbstractFFTs.jl defines
*(::Plan{T}, ::AbstractArray)which copies the real vector to aVector{T}which then is passed to the*(cFFTWPlan{T,K,true}, StridedArray{T})method in FFTW.jl which now calculates and return the complex FFT for the complex vector without ever knowing that it should mutate the vector it has been passed.Issue #341 has an MWE which shows what happens.
This PR changes the signature of
*(cFFTWPlan{T,K,true}, StridedArray{T})to*(cFFTWPlan{T,K,true}, AbstractArray). Now this method is directly invoked and throws as expected. A test for this behavior has been added.Fixes #341.