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

FFT refactoring #4985

Merged
merged 3 commits into from
Aug 21, 2024
Merged

FFT refactoring #4985

merged 3 commits into from
Aug 21, 2024

Conversation

jngrad
Copy link
Member

@jngrad jngrad commented Aug 20, 2024

Fixes #4963
Follow-up to #4969

Description of changes:

  • separate FFT backend from FFT buffers with type erasure
  • extract FFT packing functions

@jngrad jngrad marked this pull request as ready for review August 20, 2024 21:51
@jngrad jngrad requested a review from RudolfWeeber August 20, 2024 21:51
@RudolfWeeber
Copy link
Contributor

That's a big improvement in clarity for the FFT business. Thanks alot!

Looking at the FFt buffers, my impression is that scalar and vecotr buffers are strucurally identical, in that the vector buffer is just 3 scalar buffers in a row. If that is so, we might actaully just have a scalar buffer in the future and let P3Ms decide, how many of those htey want.
This might be necessary to improve the foce calculaiton in the dipolar p3m (3p3m.cpp line 404 and below).
As an aside, the d_rs assignment in line 425 looks like the inverse of the k-space permutations, just hardcoded. That might no longer be correct when replacing the underlying fft.

@jngrad
Copy link
Member Author

jngrad commented Aug 21, 2024

As an aside, the d_rs assignment in line 425 looks like the inverse of the k-space permutations, just hardcoded. That might no longer be correct when replacing the underlying fft.

I have the same feeling about that line. In fact, it took me a while to figure out why my 3D vector had permuted x/y/z axes in an earlier version of the code. I was passing the ks_pnum integer by copy rather than by reference, so the FFT initialization function wasn't updating the P3M params struct with the correct value of ks_pnum. In the current code, ks_pnum is a return value.

@kodiakhq kodiakhq bot merged commit 030d3c8 into espressomd:python Aug 21, 2024
10 checks passed
@jngrad jngrad deleted the p3m branch August 21, 2024 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

P3M: further FFt refactoring
2 participants