Skip to content
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

Adjoint of AbstractFFTs.Plan is too general #899

Open
MikaelSlevinsky opened this issue Feb 11, 2021 · 1 comment
Open

Adjoint of AbstractFFTs.Plan is too general #899

MikaelSlevinsky opened this issue Feb 11, 2021 · 1 comment

Comments

@MikaelSlevinsky
Copy link

These two invocations of the adjoint macro seem a little too general given it's not true for the (unitary) DCT plan FFTW.DCTPlan{Float64,5,false} <: AbstractFFTs.Plan{Float64}

Zygote.jl/src/lib/array.jl

Lines 729 to 741 in 956cbcf

@adjoint function *(P::AbstractFFTs.Plan, xs)
return P * xs, function(Δ)
N = prod(size(xs)[[P.region...]])
return (nothing, N * (P \ Δ))
end
end
@adjoint function \(P::AbstractFFTs.Plan, xs)
return P \ xs, function(Δ)
N = prod(size(Δ)[[P.region...]])
return (nothing, (P * Δ)/N)
end
end

julia> using FFTW, LinearAlgebra

julia> Id = Matrix{Float64}(I, 10, 10);

julia> p = plan_dct(Id, 1)
FFTW DCT (DCT-II) plan for 10×10 array of Float64

julia> P = p*Id;

julia> N = prod(size(Id)[[p.region...]])
10

julia> P'  N*inv(P)
false

It's probably false for some of the real-to-real plans as well.

@maximilian-gelbrecht
Copy link

This is also related to JuliaMath/FFTW.jl#182 as the adjoint is actually not working for ScaledPlans like some FFTs due to them having no region field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants