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

Add FFT implementation #589

Draft
wants to merge 1 commit into
base: fda/MPI2
Choose a base branch
from
Draft

Conversation

sanatgp
Copy link

@sanatgp sanatgp commented Feb 25, 2025

No description provided.

@sanatgp sanatgp marked this pull request as draft February 25, 2025 16:07
struct IRFFT! end
struct IFFT! end

export FFT, RFFT, IRFFT, IFFT, FFT!, RFFT!, IRFFT!, IFFT!, fft, ifft
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export FFT, RFFT, IRFFT, IFFT, FFT!, RFFT!, IRFFT!, IFFT!, fft, ifft

We shouldn't need to export any of these. If users want to use Dagger's FFT, all they should need to do is using Dagger, AbstractFFTs and then fft(A) where A <: DArray.

@@ -0,0 +1,576 @@
using AbstractFFTs
using LinearAlgebra
using Dagger: DArray, @spawn, InOut, In
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
using Dagger: DArray, @spawn, InOut, In

By being in Dagger, these are already available directly.

else
throw(ArgumentError("Unknown transform type"))
end

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

Comment on lines +6 to +9
struct FFT end
struct RFFT end
struct IRFFT end
struct IFFT end
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
struct FFT end
struct RFFT end
struct IRFFT end
struct IFFT end

See below for explanation.

using AbstractFFTs
using LinearAlgebra
using Dagger: DArray, @spawn, InOut, In
using MPI
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
using MPI

We won't directly use MPI for the FFT implementation - Datadeps will handle this for us automatically, when necessary.

end
end

redistribute_x_to_y!(A, B)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
redistribute_x_to_y!(A, B)
copyto!(A, B)

Dagger.@spawn name="apply_fft!(dim 2)[$idx]" apply_fft!(InOut(B_parts[idx]), In(transforms[2]), In(dims[2]))
end
end

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

end
end

end
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
end
end

end
end

redistribute!(A, B)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
redistribute!(A, B)
copyto!(A, B)

end
end

function ifft(A::DArray{T,2}, B::DArray{T,2}, transforms, dims) where T
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function ifft(A::DArray{T,2}, B::DArray{T,2}, transforms, dims) where T
function AbstractFFTs.ifft!(out::DMatrix{T}, A::DMatrix{T}; dims, decomp::Decomposition=Slab()) where T

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

Successfully merging this pull request may close these issues.

2 participants