Skip to content

Modernise TPM_FFTW#405

Open
wdeconinck wants to merge 4 commits into
developfrom
modernise_tpm_fftw
Open

Modernise TPM_FFTW#405
wdeconinck wants to merge 4 commits into
developfrom
modernise_tpm_fftw

Conversation

@wdeconinck
Copy link
Copy Markdown
Collaborator

Description

  1. This contribution moves the calls to the legacy FFTW API (SFFTW_* and DFFTW_*) to the FFTW3 API (FFTW_* and FFTWF_*).
  2. Further an extensive refactoring and documentation has been added in order to improve maintenance, and especially clarity. Duplicate code paths in EXEC_EFFTW and EXEC_FFTW have been removed. The only difference was the transposition of the PREEL argument.
  3. Magic constants like 123456 and 999999 have extracted as module parameters.
  4. Generic interfaces for FFTW functions have been added which avoid code like IF(JPRB==JPRD) obfuscating the control flow.

Contributor Declaration

By opening this pull request, I affirm the following:

  • All authors agree to the Contributor License Agreement.
  • The code follows the project's coding standards.
  • I have performed self-review and added comments where needed.
  • I have added or updated tests to verify that my changes are effective and functional.
  • I have run all existing tests and confirmed they pass.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR modernises tpm_fftw.F90 by switching from the legacy FFTW Fortran interface (SFFTW_*/DFFTW_*) to the FFTW3 iso_c_binding interface (FFTW_*/FFTWF_*). It also performs an extensive readability refactor: the two near-identical executors EXEC_FFTW and EXEC_EFFTW are unified into a single EXEC_FFTW_IMPL parameterised by a LD_TRANSPOSED flag, magic literals are pulled out as module parameters, and small precision-dispatch wrappers and generic interfaces remove inline IF(JPRB==JPRD) branches. Plan handles change type from INTEGER(KIND=JPIB) to TYPE(C_PTR) accordingly.

Changes:

  • Replace legacy FFTW API with the FFTW3 iso_c_binding API, including new TPM_FFTW_* precision-dispatch helpers and generic INTERFACE blocks; change plan handle type from INTEGER(JPIB) to TYPE(C_PTR).
  • Deduplicate EXEC_FFTW/EXEC_EFFTW into EXEC_FFTW_IMPL driven by LD_TRANSPOSED; factor input/output staging into COPY_PREEL_TO_ZFFT, COPY_ZFFT_TO_PREEL, and rank-1 variants.
  • Introduce named constants (NTYPE_C2R, NTYPE_R2C, TPM_FFTW_PLAN_FLAGS, NPLAN_ID_UNINITIALISED, NPLAN_ID_DESTROYED) and add Doxygen-style documentation throughout.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@wdeconinck wdeconinck marked this pull request as ready for review May 20, 2026 15:02
@wdeconinck wdeconinck requested a review from samhatfield May 20, 2026 15:02
@wdeconinck wdeconinck changed the title Modernise tpm fftw Modernise TPM_FFTW May 21, 2026
@wdeconinck wdeconinck marked this pull request as draft May 27, 2026 13:32
@wdeconinck
Copy link
Copy Markdown
Collaborator Author

It came to my attention that there is a FFTW API for FFT992 here: https://github.com/pmarguinaud/fftw992 which does not implement the modern FFTW API. I will adapt this PR to still use the legacy API.

Comment thread src/trans/cpu/internal/tpm_fftw.F90 Outdated
Comment thread src/trans/cpu/internal/tpm_fftw.F90 Outdated
Comment thread src/trans/cpu/internal/tpm_fftw.F90 Outdated
Comment thread src/trans/cpu/internal/tpm_fftw.F90 Outdated
@samhatfield
Copy link
Copy Markdown
Collaborator

samhatfield commented May 28, 2026

Just had a brief look. Looks good and very nice work. Let me know when you want a full review by marking the PR as ready.

@wdeconinck wdeconinck force-pushed the modernise_tpm_fftw branch from f08f4b2 to edbeac6 Compare May 28, 2026 12:49
@wdeconinck
Copy link
Copy Markdown
Collaborator Author

TPM_FFTW has been adapted to use the LEGACY interface again.
There is a hardcoded #define LEGACY_FFTW_INTERFACE 1 in tpm_fftw.F90 which can be toggled 0/1.
Once we're confident to switch to the MODERN FFTW interfaces, we can easily toggle this, and remove the added code.

@wdeconinck wdeconinck force-pushed the modernise_tpm_fftw branch from edbeac6 to 6ef0edf Compare May 28, 2026 13:09
@wdeconinck wdeconinck marked this pull request as ready for review May 28, 2026 13:11
@wdeconinck
Copy link
Copy Markdown
Collaborator Author

@samhatfield This is now ready for final review.
@pmarguinaud I have verified this compiles and runs with https://github.com/pmarguinaud/fftw992

@wdeconinck wdeconinck force-pushed the modernise_tpm_fftw branch from 6ef0edf to 42f1238 Compare May 29, 2026 07:33
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.

3 participants