espressomd / espresso

The ESPResSo package
https://espressomd.org
GNU General Public License v3.0
226 stars 183 forks source link

FFT refactoring #4985

Closed jngrad closed 1 month ago

jngrad commented 1 month ago

Fixes #4963 Follow-up to #4969

Description of changes:

RudolfWeeber commented 1 month ago

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 commented 1 month ago

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.