Sink iact overhauled to match BHs

Here's my attempt at overhauling the sink interactions such that they match the infrastructure for all the other particle types. I've used the BHs as a model, as right now the sinks only have swallow interactions.

I think I've made things consistent with the BHs, and the code compiles and appears to successfully run the Plummer sphere example simulation with the GEAR model.

Here's what I've changed:

• All self/pair/sub interaction functions have been moved from runner_sinks.h to a new file runner_doiact_functions_sinks.h for consistency with BHs.
• These functions are now declared in runner_doiact_sinks.h by pasting names around in the same way as the BHs are. All functions have been re-named accordingly.
• I'm now importing these functions in runner_main.h and the new file runner_doiact_sinks.c as appropriate, and I added the relevant lines to runner_iact_undef.h.
• I moved the declarations of the swallowing functions (e.g. runner_do_sinks_gas_swallow_self) from runner_doiact_sinks.h to runner.h for consistency with the BHs.

Please let me know if there's anything I've missed or that I should change. I requested a review from @matthieu but @Roduit please do have a look as well! For now I'm going to take this new code layout and try porting in the density interactions on a new branch.

requested review from @matthieu

assigned to @matthieu

requested review from @Roduit and removed review request for @matthieu

added sink label

Many thanks for taking care of this!

I’d suggest trying the HomogeneousBox example with GEAR sink (and grackle_0 for cooling) with a high number of particles, e.g. level 8 (N_particles = 2^(3*level) in this example).

The PlummerSphere is great for checking that we broke nothing at first sight, and the HomogeneousBox is great for checking that all sink tasks/loops are fine. I’m using the latter for all intense sink checks and debugging.

added 2 commits

• b2c7c1c7 - Move runner_do_prepare_part_sink_formation declaration into runner.h
• e9ff363d - Tidy up

Compare with previous version

mentioned in merge request !1943 (merged)

Hey again - I ran the HomogeneousBox example at level 7 (just for a bit more speed as level 8 was extremely slow). All seems to have run fine with no crashes etc.

I don't know if you want to sort out !1943 (merged) before merging this, but let me know when you're happy with it and I can run the formatter before we merge.

In the meantime I'm going to branch off this branch and try porting the density tasks in from the BHs.

Great!

First, we will finish !1943 (merged) and then attack the sink MPI !1975 (closed). Maybe we can merge this MR before the sink MPI.

We will probably merge the sink MPI before the density tasks branch. When you run the tests with the new density tasks, use --enable-debugging-checks at configure time. For testing, your best friend is the level 8 HomogeneousBox example ;)

Can I confirm what is meant by "density tasks from the BHs" ?

Sorry I should have been a bit more specific - I mean the density interactions. So right now the sinks just have swallow interactions, but I want to enable the density ones so that I can implement Bondi-Hoyle accretion for sinks consistently with how the BHs do it.

With the updates from this PR, I think this just involves making a new function runner_iact_nonsym_bh_gas_density in the Default and GEAR versions of the sink_iact.h script, and importing them in runner_doiact_sinks.c. I'm not sure if I need to do anything involving "ghost" tasks (not entirely sure I fully understand what these do other than checking that the smoothing lengths are sensible, which we don't need for the sinks).

@jdavies Notice that the last few MR added a few conflicts that need to be fixed.

mentioned in merge request !2018 (merged)

@Roduit Thanks for this. As you've probably seen I just opened up a pretty sizeable PR (!2018 (merged)) for the density tasks - I think I've got it working, but had to change a few things along the way that we can all discuss.

added 20 commits

• e9ff363d...b02fff2d - 19 commits from branch master
• 78224f30 - Merge branch 'master' into sink_iact_update

Compare with previous version

Can I clarify whether this branch is bringing new functionalities? Or is it a "straight" port of the sink routines to the more common pattern?

added 3 commits

• 66b70400 - Pluralise IACT function names
• d586a692 - Cleanup
• c0ab8f57 - Remove checks on gas for non-swallow sink tasks

Compare with previous version

added 1 commit

• 96492274 - Update undef

Compare with previous version

@Roduit The latest set of commits should have resolved all comments.

Great! Many thanks!

Can you format the code, please?

added 1 commit

• 34fc4f15 - Ran formatter

Compare with previous version

added 1 commit

• 49cd7dda - Fix erroneous references to sinks in code comments

Compare with previous version

@Roduit OK I think we're good to go - I've been running this latest version on the HomogeneousBox example for almost a day now and no problems have cropped up - I can just leave it going for a while.

Just to let you know, after today I'm going to be on leave for a week so won't be responding on here.

Great! Thank you for the job you did!

Everything is good for me. I don't know if @matthieu has anything else to suggest.

I will look at it. Are either of you expecting more changes still?

resolved all threads

That all looks fine to me. Can I confirm that @Roduit is happy with the changes and that it does not break tests?

Good cleanup

I confirm I'm happy and that tests are not broken. :)

Many thanks, @jdavies, for this!

merged

mentioned in commit 81fa7127