Recurse less in sort

Implements limiting the sort recursion depth to avoid cells that will never have a hydro interaction within or below them.

This will probably not impact the sort task speed, but should have an effect on the memory used for sort buffers.

@pdraper, can you run a quick benchmark of this change to look at memory size and CPU usage? Thanks!

added 6 commits

• 550db41e...0c86a0ae - 5 commits from branch master
• d369a115 - Merge branch 'master' of https://gitlab.cosma.dur.ac.uk/swift/swiftsim into recurse_less_in_sort

Compare with previous version

This is going to break if the hydro and star recursion depths are not the same.

Ah, good to know!

Can you give me a list of all task types/sub-types that rely on the sorted arrays being up-to-date?

Anything in runner_doiact.h and runner_doiact_stars.h

So basically any task with sub-type task_subtype_density or task_subtype_stars_density?

Yes.

OK, thanks for clarifying!

changed this line in version 5 of the diff

added 1 commit

• f0639e03 - rename flag to 'requires_hydro/starts_sorts' to make it more general.

Compare with previous version

I'm ran an EAGLE_25/.1 with a couple of ranks on this branch and the master and I think it is fair to say the differences are larger than the effect. The CPU times (non-optimized runs) are the same and we see only small changes in the numbers of allocations and size of allocations. For instance here are plots of the size as histograms for the zero (fake) and 1st steps:

and the same plots as cumulative sums:

I have others (mostly small steps, which seem identical), but I think you get the point.

Thanks, Peter, for running the numbers!

If I understand them correctly though, peak memory use, which is the limiting factor, is 30% smaller?

Only on that 1st step (2nd plots), the 0th step, which is usually our peak memory step, we're a little larger.

OK, I really don't understand how this change could allocate more memory :/

Could the metis decomposition have been different and lead to a different number of particles on that node?

Yes, probably the initial partition in this case. These are plots for just one rank. I'll run up the totals for all ranks when I get a moment.

Should have done this first. When I combine all the allocations for hydro sorts from both ranks, the memory use is pretty much identical. Just to be sure, we do expect a change in the number and size of hydro.sort labelled memory?

Yes, we should, unless we have hydro tasks all the way down the tree.

Are you running with gravity? If not, then the tree structure may well just end where the hydro interactions end and then we really shouldn't see any difference.

I have gravity and stars:

mpirun -np $SLURM_NTASKS ../../swift_mpi --pin --cosmology --hydro --self-gravity --stars -t 8 eagle_25.yml -n 32

Ran both tests once more for sanity's sake and they give the same result.

Yes, we should, unless we have hydro tasks all the way down the tree.

Sounds like the same situation I was in when attempting the same fix. And I am still confused as to why this is the case.

added 1 commit

• 6ffc7e4d - Report unnecessary recursions, note that this now no longer avoids them, it just reports them.

Compare with previous version

@pdraper, I made a quick change to report cases in which the code is recursing when it shouldn't have to. Can you run it and see if this outputs anything? Thanks!

Ran that and it reports 29,011 occurrences for all the 30 steps I run.

Putting this in perspective there are at least 30,000,000 hydro.sort allocations in same period.

Thanks for running that!

OK, so I guess the effect isn't as big as we hoped it would be. Thoughts on how to proceed?

So we agreed you would finish the function comments and accept anyway.

added 2 commits

• 28ee0628 - clean up code and make sure we're also including task_subtype_stars_density tasks.
• 351660ef - formatting.

Compare with previous version