Skip to content

FFTW: Always initialize FFTW for users when OMP is on#4861

Merged
ax3l merged 7 commits intoAMReX-Codes:developmentfrom
WeiqunZhang:fftw_init_threads
Jan 5, 2026
Merged

FFTW: Always initialize FFTW for users when OMP is on#4861
ax3l merged 7 commits intoAMReX-Codes:developmentfrom
WeiqunZhang:fftw_init_threads

Conversation

@WeiqunZhang
Copy link
Copy Markdown
Member

Close #4856

@WeiqunZhang WeiqunZhang requested review from atmyers and ax3l December 20, 2025 19:47
Copy link
Copy Markdown
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

Thank you! This looks great, but I have a few questions about:

  • external init/finalize
  • small update to third-party requirements (CMake)
  • macro guards w/o FFT option enabled

Probably all quick to resolve next week :)

@ax3l ax3l self-assigned this Jan 2, 2026
@ax3l ax3l added performance dependencies Pull requests that update a dependency file bug labels Jan 2, 2026
LIBRARIES += -Wl,-rpath,$(FFTW_HOME)/lib
endif
ifeq ($(USE_OMP),TRUE)
LIBRARIES += -lfftw3f_omp -lfftw3_omp
Copy link
Copy Markdown
Member

@ax3l ax3l Jan 5, 2026

Choose a reason for hiding this comment

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

If you like to 100% mirror the CMake logic:

Suggested change
LIBRARIES += -lfftw3f_omp -lfftw3_omp
DEFINES += -DAMREX_FFTW_OMP=1
LIBRARIES += -lfftw3f_omp -lfftw3_omp

It's a define, keep to style.
Copy link
Copy Markdown
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

LGTM, a small minor suggestion for a simpler/explicitly controlled FFTW w/ OpenMP macro.

@ax3l ax3l enabled auto-merge (squash) January 5, 2026 21:31
@ax3l ax3l merged commit 237386c into AMReX-Codes:development Jan 5, 2026
73 checks passed
@WeiqunZhang WeiqunZhang deleted the fftw_init_threads branch January 5, 2026 22:36
ankithadas pushed a commit to ankithadas/amrex that referenced this pull request Jan 7, 2026
@ax3l
Copy link
Copy Markdown
Member

ax3l commented Feb 6, 2026

@WeiqunZhang I just realized if I pass -DAMReX_FFTW_IGNORE_OMP=ON then I allow non-threaded FFTWs to build, which means we will need an internal macro like AMREX_FFTW_OMP or AMREX_USE_FFTW_OMP after all to guard the symbols/init logic.

Will PR in #4941

ax3l added a commit that referenced this pull request Feb 12, 2026
## Summary

In some corner cases, we sometimes work around broken FFTW installs: for
some (temporary) reason we cannot have any threading in FFTW, neither
with OpenMP nor threads.

In those cases, when we still want to thread the rest of AMReX with
OpenMP, we can set our AMReX transitive FFTW OpenMP requirements
explicitly off, e.g., via CMake `-DAMReX_FFTW_IGNORE_OMP=ON`. But this
is not yet implemented consistently and since #4861 exposes undefined
FFTW symbols when used.

This enables this work-around.

- [x] macro
- [x] CMake
- [x] GNUmake
- [x] `AMReX_Config.H`

## Additional background

#4861 (comment)

## Checklist

The proposed changes:
- [x] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX
users
- [ ] include documentation in the code and/or rst files, if appropriate

---------

Co-authored-by: Weiqun Zhang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug dependencies Pull requests that update a dependency file performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AMReX FFT: FFTW w/ OpenMP Threads

2 participants