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

Hide FFT implementation details #4947

Merged
merged 2 commits into from
Jul 4, 2024
Merged

Hide FFT implementation details #4947

merged 2 commits into from
Jul 4, 2024

Conversation

jngrad
Copy link
Member

@jngrad jngrad commented Jul 3, 2024

Fixes #4945

Description of changes:

  • fully encapsulate the historic FFT implementation
    • electrostatics and magnetostatics now share the same FFT operations
    • FFT operations and mesh buffers are now better documented
    • all temporary mesh buffers are now private data members
  • provide a type-erased API for the FFT backend
    • the P3M algorithm no longer has visibility of the FFT library
    • new FFT libraries can be used in place of the historic FFT implementation
  • bugfixes:
    • the P3M algorithm no longer leaks memory when FFT plans are discarded
    • the FFT implementation now keeps the MPI environment alive until all FFT plans have been destroyed
      • avoids a race condition, since the FFT plan destructor need a MPI communicator

Encapsulate the FFT algorithm and move its sources to a new folder.
Reconcile electrostatics and magnetostatics FFT operations. Hide
FFTW3 with type erasure. Alternative FFT backends can now be used.
@jngrad jngrad requested a review from RudolfWeeber July 3, 2024 22:48
RudolfWeeber
RudolfWeeber previously approved these changes Jul 4, 2024
@jngrad jngrad marked this pull request as ready for review July 4, 2024 10:12
@jngrad jngrad added this to the ESPResSo 4.3.0 milestone Jul 4, 2024
@jngrad
Copy link
Member Author

jngrad commented Jul 4, 2024

@RudolfWeeber I realized just now we could improve separation of concerns in the P3M base struct further by moving the dipolar flag to the FFT class. Now it's ready to be merged.

@rodrigoacb We have disentangled the FFT logic from the P3M logic. There is still a small overlap due to how P3M communication buffers need to know about the FFT, but everything is encapsulated into the FFTBackendLegacy class. Alternative FFT libraries can be introduced via the CMake FetchContent mechanism and the FFTBackend pure virtual base class.

@jngrad jngrad requested a review from RudolfWeeber July 4, 2024 11:40
@jngrad jngrad added the automerge Merge with kodiak label Jul 4, 2024
@kodiakhq kodiakhq bot merged commit e4d63b2 into espressomd:python Jul 4, 2024
10 checks passed
@jngrad jngrad deleted the fft branch July 4, 2024 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

P3M: further refactoring
2 participants