Fully implement the timestep limiter (non MPI)

This is a large new chunk of code. This implements the following changes:

• Implement the time-step limiter condition based on differences in time-bin and not on differences in the signal velocity,
• Move the time-step limiter loop after the time-step task,
• Add a synchronization runtime flag (--sync) that switches on the synchronization of particles hit by feedback to the current time,
• Move the time-step limiter infrastructure out of the individual hydro schemes and into its own set of files,
• Make the time-step limiter loop a loop of type "1" and not type "2" as required by the original scheme.

Implements #196 (closed).

Sorry in advance for the large diff... The MPI flavour of this will follow shortly.

changed title from Fullt implement the timestep limiter to Fully implement the timestep limiter (non MPI)

changed the description

Probably a check broken, but when I run the 3D sodshock I get:

[00000.2] engine_init_particles: Setting particles to a valid state...
[00000.2] engine_init_particles: Computing initial gas densities.
[00000.2] space_rebuild: (re)building space
[00002.3] engine_init_particles: Converting internal energy variable.
[00002.3] engine_init_particles: Running initial fake time-step.
[00002.3] space_rebuild: (re)building space
[00004.3] timestep_limiter.h:timestep_limiter_end_force():50: Minimal time-bin not computed!

Built with --enable-debugging-checks, and ran with and without --limiter, that makes no difference.

Ah thanks! Indeed this check fails incorrectly on the 0th step...

added 1 commit

• 7837b22a - Fix a debugging check that is invalid on the 0th step.

Compare with previous version

Gets a bit further...:

#   Step           Time Scale-factor     Redshift      Time-step Time-bins      Updates    g-Updates    s-Updates    b-Updates  Wall-clock time [ms]  Props
       0   0.000000e+00    1.0000000    0.0000000   0.000000e+00    1   56       589824            0            0            0              5004.489     25
[00006.3] engine_drift_all: Drifting all to t=7.812500e-04
[00006.3] space_rebuild: (re)building space
[00008.5] timestep_limiter.h:timestep_limiter_end_force():50: Minimal time-bin not computed!

So something more fundamental is going on.

What example is that?

Ah sorry... I misread. I fixed the Sedov and not the Sod shock... Although the fix should have been more general. I'll take a deeper look.

added 1 commit

• 27097ab6 - Only check the calculation of the minimal neighbour time-bin if we are not using…

Compare with previous version

And I also missed that this was using the vectorized hydro code, which was the issue here. The test now runs to completion when configuring with ./configure --enable-debugging-checks with and without the limiter runtime parameter on.

So far looks good. Ran various tests along with an EAGLE_25/.1 for 20,000 steps and they all work. Do we have some documentation for the scope and applicability of this feature?

Excellent thanks. I'll write some documentation, yes.

@jborrow this is the branch to test.

I've ran two tests here, the Sedov Blast in 2D (it is much cleaner than its 3D counterpart) and my FeedbackEvent_3D test. The Sedov looks reasonable, but you are not going to like the FeedbackEvent_3D results. All tests were completed with AnarchyDU hydrodynamics.

Sedov

Here we see a slight increase in scatter in both the pressure and velocity distributions. This is probably nothing to worry about, but it is not ideal.

Feedback Event

As a reminder, this simulates a single EAGLE feedback event and looks at what happens to the energy fractions as a function of time.

The new limiter performs particularly poorly on this test, with the energy remaining in the kinetic phase similar to the un-limited approach. This corresponds to what we have seen in the star formation histories of full cosmological runs, where the new limiter v.s. no limiter reproduce similar results.

Unfortunately, we don't really know what went on in the original EAGLE code on these tests, and I'm not volunteering to test that. However, is there a chance that we have made this limiter too un-aggressive?

My main concern here is that we see that the Sedov does well, but not the feedback event. This makes me think that the reason that we do poorly with the new limiter here is not actually anything to do with the hydrodynamics, but rather the cooling...

It's worth pointing out here that the new limiter takes half as many steps as the old limiter. It's also worth noting that the 'Old Limiter' result is nearly converged with time-step, but the 'New Limiter' result is not - if I reduce the CFL to 0.025 then I recover the results from the old limiter.

For reference, here is the converged result:

But keep in mind that the old limiter was wrong.

Also did you add --sync to your test?

One other thing is that you can make the limiter more aggressive by setting time_bin_neighbour_max_delta_bin to 1 in src/timeline.h.

added 1 commit

• 41b0438b - Removed unused fictious time-bin

Compare with previous version

added 1 commit

• 5d405d0a - Added the new runtime option to the documentation.

Compare with previous version