Fix rare removing particles bug

Fix a weird, rare crash that occasionally appears when removing a particle in the same step as writing a snapshot.

added 1 commit

• fe53e3b1 - Fix Tillotson units

Compare with previous version

added 1 commit

• 1c527741 - Remove debug code

Compare with previous version

unmarked as a Work In Progress

marked as a Work In Progress

changed the description

unmarked as a Work In Progress

changed the description

Ok. More than just that change. I can merge the other changes straightaway as they only affect planetary physics. For the particle removing, more tests will be needed.

added 64 commits

• 1c527741...13f05c12 - 63 commits from branch master
• 2d2922e9 - Merge branch 'master' into lock_before_removing_particles_debug

Compare with previous version

No rush since it seems to work for me on the branch as is. And most of the changes are just aesthetic or typos apart from this fix. I guess we don't know exactly which fix attempts were needed in the end...

Let me know if I can help with testing

added 2 commits

• 45c79942 - Applied code formatting tool.
• a9bfc4a6 - Applied the same checks for already removed particles to the other species.

Compare with previous version

added 2 commits

• f384bd3f - 1 commit from branch master
• c7f10f7c - Merge branch 'master' into lock_before_removing_particles_debug

Compare with previous version

I have merged all the non-removing particle changes into master. I now need to apply the same loacking changes to the other particle species and specifically test that it works fine for the BH swallowing.

added 2 commits

• 776f567e - Apply the same changes to the other places where particles get removed.
• 31aed3eb - When removing an spart or a bpart, update the counters accordingly.

Compare with previous version

@jkeger could you cross-check that my latest changes did not break anything on your side? I have become paranoid...

assigned to @pdraper

@pdraper I think this is all fine but since we have been scratching our head a lot on this with Jacob, I'd appreciate an additional look.

The idea is that the cell_remove_xxxx now also remove the gpart friend. This is to avoid any issues of double-removing and thread synchronization.

All seems to make sense to me, don't see any race conditions with removing the gpart at the same time as the other parts, guessing that was the problem before (the type check nicely stops that). So good to go I'd say.

I can run some repeat tests later this week. I'd say it looks like the fixes haven't changed, but like you say it's not like we truly understood it at any point, so better safe than sorry

Thanks Peter. I think I'll accept and if Jacob's runs show something odd happening again, we'll advise.

merged

mentioned in commit 68aef9ad