Skip to content

Pr501 fft final#570

Merged
aaadelmann merged 3 commits into
masterfrom
pr501-fft-final
Jul 21, 2026
Merged

Pr501 fft final#570
aaadelmann merged 3 commits into
masterfrom
pr501-fft-final

Conversation

@aaadelmann

@aaadelmann aaadelmann commented Jul 20, 2026

Copy link
Copy Markdown
Member
## Summary

This PR ports the FFT backend refactor from the larger PR501 branch onto current `master`, keeping the scope limited and reviewable.

The main change is to split the former monolithic FFT implementation into smaller backend and transform modules while preserving the existing public FFT usage for the current HeFFTe-backed transforms.

## What Is Included

### FFT Core Refactor

The FFT implementation is now organized into separate headers:

- `src/FFT/Traits.h`
- `src/FFT/Backend/Backend.h`
- `src/FFT/Backend/Heffte.h`
- `src/FFT/Transform/Common.h`
- `src/FFT/Transform/CC.h`
- `src/FFT/Transform/RC.h`
- `src/FFT/Transform/Trig.h`
- `src/FFT/Transform/Transform.h`

`src/FFT/FFT.h` is now the aggregate include for the refactored FFT module.

The old monolithic implementation header:

- `src/FFT/FFT.hpp`

was removed.

### Pruned Transforms

This PR also adds the pruned transform specializations:

- `src/FFT/Transform/PrunedCC.h`
- `src/FFT/Transform/PrunedRC.h`

These are included through the FFT transform aggregate header.

### Optional cuFFTMp Backend

This PR adds the multi-node NVIDIA cuFFTMp backend as an explicitly opt-in backend:

- `src/FFT/Backend/CuFFTMp.h`

A new CMake option was added:

```cmake
-DIPPL_ENABLE_CUFFTMP=ON

The option is guarded so that cuFFTMp can only be enabled when FFT and CUDA are both enabled:

  • IPPL_ENABLE_CUFFTMP=ON requires IPPL_ENABLE_FFT=ON
  • IPPL_ENABLE_CUFFTMP=ON requires CUDA in IPPL_PLATFORMS

The backend header is included only when both conditions are active:

#if defined(IPPL_ENABLE_CUFFTMP) && defined(KOKKOS_ENABLE_CUDA)
#include "FFT/Backend/CuFFTMp.h"
#endif

Plain single-node CuFFT.h support is intentionally not included in this PR.

What Is Intentionally Not Included

The original pr501-fft branch also contained changes to FFTOpenPoissonSolver and solver tests. Those were reviewed but not ported.

Reason: current master already contains newer Poisson solver functionality, including integrated and shifted Green-function support. The corresponding hunks from pr501-fft would remove that current coverage and regress master.

Specifically, this PR does not remove:

  • GreenFunction::{STANDARD, INTEGRATED}
  • integrated Green-function support
  • shifted integrated Green-function support
  • TestGaussianIntegrated
  • TestIntegratedGreensFunction
  • current CG, preconditioner, and multigrid tests

Validation

Configured and built an OpenMP FFT/unit-test build:

cmake -S . -B build-pr501-fft-final-openmp \
  -DIPPL_PLATFORMS=OPENMP \
  -DIPPL_ENABLE_FFT=ON \
  -DIPPL_ENABLE_UNIT_TESTS=ON \
  -DIPPL_ENABLE_TESTS=ON

cmake --build build-pr501-fft-final-openmp -j 8
ctest --test-dir build-pr501-fft-final-openmp --output-on-failure

Result:

100% tests passed, 0 tests failed out of 70

Configured and built a solver-enabled OpenMP build:

cmake -S . -B build-pr501-fft-final-solvers-openmp \
  -DIPPL_PLATFORMS=OPENMP \
  -DIPPL_ENABLE_FFT=ON \
  -DIPPL_ENABLE_SOLVERS=ON \
  -DIPPL_ENABLE_UNIT_TESTS=ON \
  -DIPPL_ENABLE_TESTS=ON

cmake --build build-pr501-fft-final-solvers-openmp -j 8
ctest --test-dir build-pr501-fft-final-solvers-openmp --output-on-failure

Result:

100% tests passed, 0 tests failed out of 85

Also verified the cuFFTMp CMake guard:

-DIPPL_PLATFORMS=OPENMP -DIPPL_ENABLE_FFT=ON -DIPPL_ENABLE_CUFFTMP=ON

fails clearly with:

IPPL_ENABLE_CUFFTMP requires CUDA in IPPL_PLATFORMS

Scaling

image

Notes

CUDA/cuFFTMp backend code is included behind compile-time guards and was not fully exercised locally because the local build is OpenMP-only. The default path remains HeFFTe, and non-CUDA builds do not include the cuFFTMp backend header.

Split the existing monolithic FFT implementation into smaller headers:

- add FFT traits and transform/backend dispatch declarations
- move HeFFTe-specific implementation into FFT/Backend/Heffte.h
- move CC, RC, and trigonometric transform specializations into FFT/Transform/
- replace FFT.h with an aggregate include for the new module structure
- remove the obsolete FFT.hpp implementation header

This first slice intentionally keeps the scope limited to the existing HeFFTe-backed
CC, RC, sine, cosine, and cosine-I transforms. Pruned transforms and CUDA/cuFFTMp
backend files are deferred to later commits so the refactor remains reviewable.

Validation:
- configured local OpenMP build with IPPL_ENABLE_FFT=ON, IPPL_ENABLE_UNIT_TESTS=ON,
  and IPPL_ENABLE_TESTS=ON
- built build-pr501-fft-final-openmp successfully
- ran ctest --test-dir build-pr501-fft-final-openmp --output-on-failure
- all 70 tests passed, including FFT, TestFFTCC, TestFFTRC, TestSine, TestCos,
  and TestCos1
Add the pruned complex-to-complex and real-to-complex FFT transform
specializations from the PR501 FFT refactor:

- add FFT<PrunedCCTransform, ComplexField>
- add FFT<PrunedRCTransform, RealField>
- include both pruned transform headers from the FFT transform aggregate

This commit keeps pruned transforms separate from the core FFT module split and
from the CUDA backend additions, making backend and transform issues easier to
isolate during review.

Validation:
- rebuilt build-pr501-fft-final-openmp successfully
- ran ctest --test-dir build-pr501-fft-final-openmp --output-on-failure
- all 70 configured tests passed
Add the CUDA-aware cuFFTMp backend from the PR501 FFT refactor as an explicitly
opt-in backend:

- add src/FFT/Backend/CuFFTMp.h
- add the IPPL_ENABLE_CUFFTMP CMake option
- define IPPL_ENABLE_CUFFTMP only when requested
- require IPPL_ENABLE_FFT=ON when cuFFTMp is enabled
- require CUDA in IPPL_PLATFORMS when cuFFTMp is enabled
- include the cuFFTMp backend only when both IPPL_ENABLE_CUFFTMP and
  KOKKOS_ENABLE_CUDA are active

Plain cuFFT support is intentionally not added in this commit. The normal HeFFTe
path remains the default, and non-CUDA builds do not include the cuFFTMp header.

Validation:
- reconfigured build-pr501-fft-final-openmp with IPPL_ENABLE_FFT=ON and default
  IPPL_ENABLE_CUFFTMP=OFF
- rebuilt build-pr501-fft-final-openmp successfully
- ran ctest --test-dir build-pr501-fft-final-openmp --output-on-failure
- all 70 configured tests passed
- verified that configuring IPPL_ENABLE_CUFFTMP=ON without CUDA fails clearly
  with "IPPL_ENABLE_CUFFTMP requires CUDA in IPPL_PLATFORMS"
@aaadelmann
aaadelmann requested a review from srikrrish July 20, 2026 11:28
@aaadelmann aaadelmann self-assigned this Jul 20, 2026
@aaadelmann aaadelmann added enhancement New feature or request cleanup nvcc Issues related to NVCC gitlab-mirror labels Jul 20, 2026

@srikrrish srikrrish left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I just noticed that apart from the PCG PR Paul is not mentioned as author in the previous PR (HOSG final) as well as this one. This needs to be fixed. Since HOSG final is already merged it may be difficult to correct it there and not sure (sorry should have checked more carefully). but hopefully at least in this PR and in the future ones it can be corrected.

Comment thread src/FFT/Traits.h
Comment thread src/FFT/Traits.h
@aaadelmann

Copy link
Copy Markdown
Member Author

Will add Paul to this one and see if we can add him to the other as well.

@srikrrish srikrrish left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It was decided that Paul will be added as a co-author at the end of all the PRs from 501 by selecting the individual commits. Hence, I am approving now.

@aaadelmann
aaadelmann added this pull request to the merge queue Jul 21, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 21, 2026
@aaadelmann
aaadelmann merged commit 0424793 into master Jul 21, 2026
10 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup enhancement New feature or request gitlab-mirror nvcc Issues related to NVCC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants