Stars use sort

In this branch, I am optimizing the dopair functions for the stars. They are about 100 times faster than the previous version on EAGLE_6. The global speedup is about 2x.

I discovered a bug in the optimized version of the hydro. Here https://gitlab.cosma.dur.ac.uk/swift/swiftsim/blob/master/src/runner_doiact.h#L1003 we cannot use this exit condition. hi_max is not always the max, therefore we can skip a few particles (quick estimation of about 1 missed neighbors every 10'000 particles).

I am not solving it yet (will be in a separate merge request), but the stars are not reproducing this error. Due to this error, the stars sort is not as useful as expected, therefore I am also removing the sorts.

@matthieu, I am also fixing a few checks from your branch.

Currently I am requesting to merge with mpi_stars2_matthieu, but we can wait a bit longer and do it directly to the master.

added 1 commit

• 6d175861 - Fix tests

Compare with previous version

added 1 commit

• 1ff150be - Update recv_spart arguments

Compare with previous version

added 2 commits

• 4a72604c - Remove a few fields in cells
• ddbe2d20 - Fix debug-interactions-stars

Compare with previous version

Wait wait. Removing all the sorting?

Either we remove the stars sorts or we ensure that h_max is always the max. I am keeping the sort for the hydro.

Ok. We need to talk about this. That's not a small change.

changed the description

changed target branch from mpi_stars2_matthieu to master

marked as a Work In Progress

I compared the run time with and without the stars sort. On EAGLE_6, IsolatedGalaxy_starformation and a GEAR example, the run time is comparable between the two (less than 5% difference).

What do you prefer?

So, 5% is definitely not negligible. That's 250k CPU hours on an EAGLE simulation. :) With these numbers I'd prefer the sorted one.

If you have a branch with the sorted and another one with the unsorted I can benchmark both on the bigger test cases we have.

added 66 commits

• ddbe2d20...3d44fb65 - 58 commits from branch master
• efdaad51 - Start working on non naive stars interactions
• 49badaef - Clean flag UPDATE_STARS
• bb368b54 - fix configure
• 285cbdea - Stars: dopair optimized
• 9ab76ca7 - Stars: optimized dopair1 works
• 89909920 - Stars: dopair_subset optimized and works
• 510631b0 - stars: Reimplement break condition in forloop
• b021e410 - stars: skip subset if no part

Compare with previous version

Feel free to test is at large scale. I have done only small tests where 5% are more or less the accuracy of the measurement. From what I have seen, using the sort is a bit faster in most cases (but not all).

My branches are this one and stars_no_srt (without o).

Thanks. I'll give these a go.

unmarked as a Work In Progress

Took some time to get the jobs through but it looks like on EAGLE25 and EAGLE50 the sorted version is 3% faster.

Great it means that our work was not useless :)

added 1 commit

• 9268ca83 - Applied code formatting tool.

Compare with previous version

added 3 commits

• 24492780 - Apply the correction for inhibited particle in the star self loops following…
• 552dbe41 - Skip inhibited particles in the naive star pair loops.
• f17b6145 - Early in the star pair loops if there are no gas particles.

Compare with previous version

@lhausammann I made some small changes; let me know if you disagree with them.

Otherwise, this is good to go. Handing over to @pdraper for a potential last check.

assigned to @pdraper

Seems good to me

added 1 commit

• c9907eaf - Need engine_rank in this context

Compare with previous version

All looks good to me, you two still happy to merge?

Yes, all good. Thanks!

merged

mentioned in commit b892e7f5

OK, done.