Clarify the use of plan_fft!
#82
Conversation
The existing docstring refers to the array `A` that is used to construct the plan, but that array is irrelevant (other than its eltype and size) when using the plan later. The old wording made it sound like the `A` in the constructor itself is used.
Codecov ReportBase: 84.13% // Head: 84.13% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #82 +/- ##
=======================================
Coverage 84.13% 84.13%
=======================================
Files 2 2
Lines 208 208
=======================================
Hits 175 175
Misses 33 33
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
bump :) |
src/definitions.jl
Outdated
| one evaluates the FFT of an array `B` | ||
| of the same `size` | ||
| via `p * B` or via `mul!(B, p, B)`. | ||
| For the `mul!` use, `B` must have the same `eltype` as `A`. |
There was a problem hiding this comment.
Even for p * B, it won't work in-place if B doesn't have the same eltype, no?
(Note also that the strides have to be the same too, when applying a plan to a new array. This is typically only relevant for arrays that are views of other arrays, as otherwise the strides are determined by the size.)
There was a problem hiding this comment.
Actually, the p * B version works fine even if B has different eltype and strides from A:
using LinearAlgebra: mul!
using FFTW: plan_fft!
dima = (8,16)
A = Array{ComplexF32}(undef, dima)
B = rand(ComplexF16, 4, dima...)
p = plan_fft!(A)
C = @view B[1,:,:] # strides and eltype differs from A
p * C # works fine!
D = collect(C)
@assert p * C == p * D # passes You make a good point that the mul! version requires matching strides too so I updated the PR to mention that.
The existing docstring refers to the array
Athat is used to construct the plan, but that array is irrelevant (other than its eltype and size) (Edit: and strides) when using the plan later. The old wording made it sound like theAin the constructor itself is used.