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.

Sign up for GitHub

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

Fix rrule for rfft and ifft for CuArray #96

Closed
wants to merge 4 commits into from
Closed

Fix rrule for rfft and ifft for CuArray #96

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e0e667f
fix https://github.com/JuliaMath/AbstractFFTs.jl/issues/95
ziyiyin97 Mar 28, 2023
eac7da4
scaling in place instead of type conversion
ziyiyin97 Apr 11, 2023
dc2cfab
fix immutable array by similar
ziyiyin97 Apr 12, 2023
a22c168
add more comments
ziyiyin97 Apr 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 30 additions & 15 deletions ext/AbstractFFTsChainRulesCoreExt.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,24 @@ end
function ChainRulesCore.rrule(::typeof(rfft), x::AbstractArray{<:Real}, dims)
y = rfft(x, dims)

# compute scaling factors
halfdim = first(dims)
d = size(x, halfdim)
n = size(y, halfdim)

project_x = ChainRulesCore.ProjectTo(x)
function rfft_pullback(ȳ)
dY = ChainRulesCore.unthunk(ȳ) ./ 2
selectdim(dY, halfdim, 1) .*= 2
dY = ChainRulesCore.unthunk(ȳ)
# apply scaling
dY_scaled = similar(dY)
dY_scaled .= dY
dY_scaled ./= 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could actually fuse this line with the previous one I think, e.g. dY_scaled .= dY ./ 2 and similarly for the other scalings

v = selectdim(dY_scaled, halfdim, 1)
v .*= 2
if 2 * (n - 1) == d
selectdim(dY, halfdim, n) .*= 2
v = selectdim(dY_scaled, halfdim, n)
v .*= 2
end
x̄ = project_x(brfft(dY, d, dims))
x̄ = project_x(brfft(dY_scaled, d, dims))
return ChainRulesCore.NoTangent(), x̄, ChainRulesCore.NoTangent()
end
return y, rfft_pullback
Expand Down Expand Up @@ -68,19 +73,24 @@ end
function ChainRulesCore.rrule(::typeof(irfft), x::AbstractArray, d::Int, dims)
y = irfft(x, d, dims)

# compute scaling factors
halfdim = first(dims)
n = size(x, halfdim)
invN = AbstractFFTs.normalization(y, dims)

project_x = ChainRulesCore.ProjectTo(x)
function irfft_pullback(ȳ)
dX = rfft(real.(ChainRulesCore.unthunk(ȳ)), dims) .* invN .* 2
selectdim(dX, halfdim, 1) ./= 2
dX = rfft(real.(ChainRulesCore.unthunk(ȳ)), dims)
# apply scaling
dX_scaled = similar(dX)
dX_scaled .= dX
dX_scaled .*= invN .* 2
v = selectdim(dX_scaled, halfdim, 1)
v ./= 2
if 2 * (n - 1) == d
selectdim(dX, halfdim, n) ./= 2
v = selectdim(dX_scaled, halfdim, n)
v ./= 2
end
x̄ = project_x(dX)
x̄ = project_x(dX_scaled)
return ChainRulesCore.NoTangent(), x̄, ChainRulesCore.NoTangent(), ChainRulesCore.NoTangent()
end
return y, irfft_pullback
Expand Down Expand Up @@ -109,18 +119,23 @@ end
function ChainRulesCore.rrule(::typeof(brfft), x::AbstractArray, d::Int, dims)
y = brfft(x, d, dims)

# compute scaling factors
halfdim = first(dims)
n = size(x, halfdim)

project_x = ChainRulesCore.ProjectTo(x)
function brfft_pullback(ȳ)
dX = rfft(real.(ChainRulesCore.unthunk(ȳ)), dims) .* 2
selectdim(dX, halfdim, 1) ./= 2
dX = rfft(real.(ChainRulesCore.unthunk(ȳ)), dims)
# apply scaling
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe also add a comment saying something like # below approach is for ensuring GPU compatibility, see PR #96?

dX_scaled = similar(dX)
dX_scaled .= dX
dX_scaled .*= 2
v = selectdim(dX_scaled, halfdim, 1)
Copy link
Contributor

@gaurav-arya gaurav-arya Apr 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a comment above this line saying something like like # assign view to a separate variable before assignment, to support Julia <1.2?

v ./= 2
if 2 * (n - 1) == d
selectdim(dX, halfdim, n) ./= 2
v = selectdim(dX_scaled, halfdim, n)
v ./= 2
end
x̄ = project_x(dX)
x̄ = project_x(dX_scaled)
return ChainRulesCore.NoTangent(), x̄, ChainRulesCore.NoTangent(), ChainRulesCore.NoTangent()
end
return y, brfft_pullback
Expand Down