Skip to content

Use new particle data layout in the P3M code - #5137

Merged
kodiakhq[bot] merged 1 commit into
espressomd:pythonfrom
jngrad:p3m-shm
Aug 18, 2025
Merged

Use new particle data layout in the P3M code#5137
kodiakhq[bot] merged 1 commit into
espressomd:pythonfrom
jngrad:p3m-shm

Conversation

@jngrad

@jngrad jngrad commented Aug 14, 2025

Copy link
Copy Markdown
Member

No description provided.

@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@jngrad

jngrad commented Aug 15, 2025

Copy link
Copy Markdown
Member Author

With the new P3M shared memory implementation, Coulomb P3M is 17% faster and Dipolar P3M is 7% faster with 2000 particles per thread. The MPI implementation is also faster now. Weak scaling benchmarks will run over the week-end.

Use shared memory in P3M/DP3M charge/dipole/force/torque assignment
and real-space force calculation in the short-range loop.
@jngrad
jngrad marked this pull request as ready for review August 18, 2025 14:31
@jngrad

jngrad commented Aug 18, 2025

Copy link
Copy Markdown
Member Author

The benchmarks are inconclusive due to the high variance. Running the P3M benchmark with 4 threads multiple times yields two different runtimes reliably: 3.7ms/step and 4.6ms/step. Since the difference is significant, one cannot take the mean of multiple runs. Taking the minimum (or first quartile) of multiple runs requires a larger sample size than what I currently have.

kokkos_parallel_range_for<Kokkos::RangePolicy<execution_space>>(
"reduction", std::size_t{0}, unique_particles.size(),
[&local_force, &unique_particles, num_threads](std::size_t const i) {
Utils::Vector3d force{};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it is possible to apply local_force directly to unique_particles.at(i)->force(), and the calculation became a bit faster due to not generating Utils::Vector3d.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

A quick benchmark reveals no major speed-up, but I'll include that in my next PR because it improves readability.

auto pos2 = copy_aosoa_vector_elc(p2, aosoa.position);
auto pos1 = copy_aosoa_vector_elc(p1, aosoa.position);
pos1[2] *= -1.;
auto const d = box_geo.get_mi_vector(pos2, pos1);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When we use aosoa_pack, it would be faster to assign each value of aosoa.position directly to get_mi_vector(T const &a0, T const &a1, T const &a2, T const &b0, T const &b1, T const &b2) than to use get_mi_vector(const Utils::Vector<T, 3> &a, const Utils::Vector<T, 3> &b).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Here I'm leveraging compiler optimizations that elide out the temporary Vector3d objects from the stack. Verified on the Compiler Explorer using GCC 15.2 and Clang 19.1.

auto pos2 = copy_aosoa_vector_elc(p2, aosoa.position);
auto pos1 = copy_aosoa_vector_elc(p1, aosoa.position);
pos1[2] = 2. * box_h - pos1[2];
auto const d = box_geo.get_mi_vector(pos2, pos1);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as line 133.

@hidekb hidekb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this looks good to me. Thanks a lot for the great work.

@jngrad jngrad added the automerge Merge with kodiak label Aug 18, 2025
@kodiakhq
kodiakhq Bot merged commit fefe8a0 into espressomd:python Aug 18, 2025
10 checks passed
@jngrad
jngrad deleted the p3m-shm branch August 18, 2025 17:49
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.

2 participants