SWIFTSIM / HBTplus

HBTplus halo finder adapted for the FLAMINGO and COLIBRE simulations
0 stars 0 forks source link

Only use bound particles to compute halo positions and velocities #14

Closed jchelly closed 4 months ago

jchelly commented 4 months ago

I think I made a mistake in #4. When computing halo positions and velocities for merging I'm looping over all particles in the subhalo.Particles array. I think this is incorrect because the array can contain unbound particles. I should instead be looping over only indexes 0 to Nbound-1.

This will make a difference for subhalos which don't have enough tracer particles in their bound part but do have some unbound tracers.

VictorForouhar commented 4 months ago

Good catch! I think this should be incorporated into master.

Somewhat unrelated, but should we also modify https://github.com/SWIFTSIM/HBTplus/blob/66b6c5f0b789774fd2f9e63e230dd9865b4a1d59/unit_tests/test_build_pos_vel.cpp#L99 and https://github.com/SWIFTSIM/HBTplus/blob/66b6c5f0b789774fd2f9e63e230dd9865b4a1d59/unit_tests/test_build_pos_vel.cpp#L110 ?

jchelly commented 4 months ago

Yes, in the test Particles.size()==sub.Nbound so it works as it is but it's more consistent to use Nbound.