Remove engine_marktasks() entirely

This is a relic of a long-gone past where marktasks and unskip needed to be different. Now it just makes use duplicate entire code blocks for no good reasons. engine_unskip() is superior as it only loops over the tree sections where actual work could happen, so that's the version we should keep.

This version keeps marktasks as a way to verify unskip's job. Once we are happy they are identical (as they should) we can remove that check.

Actual changes to keep past these tests:

• Better names for the various BH ghosts between the task names and the cell-carried task pointers.
• More uniform activation (for readability) of the gravity drift related to MPI between unskip and marktasks.
• Remove the activation of some gas drifts when neighbouring a BH cell where the BH is not active.
• Fix a mismatch where marktasks was activating unnecessary star pairs.
• Fix a mismatch where marktasks was activating unnecessary BH ghosts.
• Fix a mismatch where markatask was not activating star drifts in the case of stars doing only gravity.
• Fix a mismatch where marktasks was not activating RT_in tasks.

added code health enhancement labels

assigned to @matthieu

marked this merge request as draft

changed the description

marked this merge request as ready

requested review from @bvandenbroucke

assigned to @pdraper and unassigned @matthieu

I have tested it on a bunch of setups and have not seen any issues. More scrutiny might be good though as it is currently a critical code section.

Low priority though as it is not a critical improvement.

added 10 commits

• 80f294e8...ee7a8c8b - 8 commits from branch master
• e5398c51 - Remove engine_marktasks() entirely
• bab47997 - Reintroduce the marktasks functions for temporary debugging checks

Compare with previous version

added 1 commit

• ccb57dd3 - First attempt at using marktasks to verify the result of unskip

Compare with previous version

marked this merge request as draft

removed enhancement label

added 97 commits

• ccb57dd3...f200b0dc - 96 commits from branch master
• e83e3937 - Merge branch 'master' into no_marktasks

Compare with previous version

added 1 commit

• 6e139fd0 - When checking the result of unskip via marktasks, first clear the cell flags

Compare with previous version

added 1 commit

• 4a56ee3e - Set the correct value for the skip filed when resetting the tasks before cheking marktasks

Compare with previous version

marked this merge request as ready

I have now implemented what you suggested.

The normal code only uses engine_unskip(). But, when using debugging checks, we switch to a mode where the code verifies the result is consistent. That means we run the regular engine_unskip() then void all the operations and run engine_marktasks(). Each task contains a flag specifying how it was unskipped. Once both modes have run, we perform a loop to verify the tasks have both flags set (or none).

This is likely only a temporary measure to verify the change to only using engine_unskip() is indeed safe.

@mivkov if you have time to verify that this branch does not break RT and in particular sub-cycling that'd be great.

changed the description

added 1 commit

• 1ec27f2f - Fix misleading error message

Compare with previous version