search

ECP-copa / Cabana

Performance-portable library for particle-based simulations
Other
188 stars 51 forks source link

Particle creation with minimum distance is not thread safe #718

Open marcel-dcs opened 5 months ago

marcel-dcs commented 5 months ago

I did not manage to use createParticles with a min_dist option (to prevent overlapping particles) when using multiple threads. At first I expected an incorrect usage on my end, but there appears to be a bug in the unit test for this functionality. The checkRandomDistances function has the following two issues:

  1. The min_dist is converted to an int, which casts the 0.47 value to 0 (which is why the check never fails) see: https://github.com/ECP-copa/Cabana/blob/master/core/unit_test/tstParticleInit.hpp#L52

  2. The inner loop over j should start at i+1, to prevent checks for i==j and duplicate checks see: https://github.com/ECP-copa/Cabana/blob/master/core/unit_test/tstParticleInit.hpp#L59

After correcting for these, the ParticleInit unit test fails. Therefore, it looks like the insertion with a minimum distance is not thread safe: the id used here, might not match the actual position where the particle is inserted in the particle_list (here). If so, some particles accepted for insertion might overlap with others inserted around the same time.

streeve commented 5 months ago

Thanks for the detailed report! I'll look into how best to fix