Mladen Ivkovic (a6223ddb) at 26 Mar 14:09
getting same results as grackle standalone
... and 82 more commits
time
wasn't updated before either, just re-computed. Most likely a copy-paste leftover on my side from what's done below.
time
is then supposed to be updated in the lower block. That's inside the actual for loop that runs the sub-cycles.
Ah, I see now what you mean. But yes, the result for
time = e->time;
and
time = e->ti_current_subcycle * e->time_base + e->time_begin;
is identical at this point, as we're printing out the stats after the "zeroth" sub-cycle, which is performed during the regular task launch. So e->time
is correct too.
It changes further below in the actual sub-cycling loop, which has the correct way of computing it for that case.
Yes. We never require the engine time, and I want to avoid touching anything in the engine during sub-cycles that doesn't need touching and isn't separated specifically for RT sub-cycling stuff.
We basically only need the time to write printouts, so that's why it's fine to keep it as a local variable here.
The only place I can think of that we'd need the time may be to determine the age of stars, or something related. However, that would mean that star particles are active, and we therefore cannot be within a sub-cycle.
Updated sub-cycling for cosmological runs with GEAR-RT
Sorry for the delay in the review. Looks good to me. Thanks!
Much better, thank you! Could you also apply the code formatter on this before we can merge it?
The correct way to compute the sub-cycling time step size that we've already passed should be dt_subcycle = cosmology_get_delta_time(cosmo, e->ti_current, e->ti_rt_end_min);
when running with cosmo, and dt_subcycle = rt_step_size * e->time_base;
when running without. In the case with cosmology, dt_subcycle
can't be const
any longer, as the physical time between two redshifts/scale factors varies with the current redshift/scale factor. So dt_subcycle
will need to be recomputed every sub-cycling loop below.
Cosmological treatment of the IonMassFractionAdvectionTest_2D
example.
Added CosmoIonMassFractionAdvectionTest_2D
that runs the ion mass fraction test at high redshift (z=100 to z=70)
Looks good, tests work and pass. Thanks!
runCosmo would be better, I'd think.
Mladen Ivkovic (9f507d28) at 08 Mar 21:40
Porting issue here from slack, with instructions for reproducible example.
Configure with:
./configure --with-hydro-dimension=1 --enable-debug --enable-debugging-checks
Reproducible example:
swiftsim/examples/RadiativeTransferTests/CosmoAdvection_1D
(yes, it's an RT example, but the debug check fail also occurs without RT.)
run with
../../../swift --hydro --cosmo rt_advection1D_medium_redshift.yml
Run crashes after final step:
[00001.8] engine_print_stats: Saving statistics at a=1.405296e-01
[00001.8] engine_dump_snapshot: Dumping snapshot at a=1.407294e-01
61 9.276920e-01 0.1398616 6.1499254 9.757966e-03 50 50 1000 0 0 0 0 8.487 24 0.101
[00001.8] cosmology.c:cosmology_get_delta_time():1294: ti_end must be >= ti_start
gdb
backtrace:
[00001.8] cosmology.c:cosmology_get_delta_time():1294: ti_end must be >= ti_start
Thread 1 "swift" received signal SIGABRT, Aborted.
0x00007ffff4c969fc in pthread_kill () from /lib/x86_64-linux-gnu/libc.so.6
(gdb) bt
#0 0x00007ffff4c969fc in pthread_kill () from /lib/x86_64-linux-gnu/libc.so.6
#1 0x00007ffff4c42476 in raise () from /lib/x86_64-linux-gnu/libc.so.6
#2 0x00007ffff4c287f3 in abort () from /lib/x86_64-linux-gnu/libc.so.6
#3 0x0000000000483bd7 in cosmology_get_delta_time (c=<optimised out>, ti_start=<optimised out>, ti_end=<optimised out>) at cosmology.c:1294
#4 0x000000000044c7d4 in engine_io_check_snapshot_triggers (e=e@entry=0x7ffffff9a638) at engine_io.c:1270
#5 0x0000000000435f0a in engine_step (e=e@entry=0x7ffffff9a638) at engine.c:2465
#6 0x0000000000408283 in main (argc=<optimised out>, argv=<optimised out>) at swift.c:1720
(gdb) frame 3
#3 0x0000000000483bd7 in cosmology_get_delta_time (c=<optimised out>, ti_start=<optimised out>, ti_end=<optimised out>) at cosmology.c:1294
1294 if (ti_end < ti_start) error("ti_end must be >= ti_start");
(gdb) p ti_end
$1 = <optimised out>
(gdb) p ti_start
$2 = <optimised out>
(gdb) frame 4
#4 0x000000000044c7d4 in engine_io_check_snapshot_triggers (e=e@entry=0x7ffffff9a638) at engine_io.c:1270
1270 time_to_next_snap = cosmology_get_delta_time(e->cosmology, e->ti_current,
(gdb) l
1265 const int with_cosmology = (e->policy & engine_policy_cosmology);
1266
1267 /* Time until the next snapshot */
1268 double time_to_next_snap;
1269 if (e->policy & engine_policy_cosmology) {
1270 time_to_next_snap = cosmology_get_delta_time(e->cosmology, e->ti_current,
1271 e->ti_next_snapshot);
1272 } else {
1273 time_to_next_snap = (e->ti_next_snapshot - e->ti_current) * e->time_base;
1274 }
(gdb) p e->ti_current
$3 = 139611588448485376
(gdb) p e->ti_next_snapshot
$4 = -1
Stan's analysis:
What I believe is happening is that when using a snapshot file, and you have reached the last snapshot, the ti_next_snapshot is explicitly set to -1. However, when running with debugging checks, if ti_next_snapshot is negative, it throws an error. I don't remember the exact files/lines where it happens, but the -1 is deliberately set in one of the files that checks for snapshot times in a snapshot list.
how many sub-cycles did you run with? Did sub-cycles actually run?
Mladen Ivkovic (9f507d28) at 08 Mar 19:04
add Stan to authors list
Test compiles and runs, results look good.
However, I don't think we need a whole new example directory for this. I would suggest to merge the cosmo example and the non-cosmo example:
makeIC.py
you made for the cosmology case, and move that into examples/RadiativeTransferTests/IonMassFractionAdvectionTest_2D
advect_ions.yml
file to IonMassFractionAdvectionTest_2D/advect_ions_cosmo.yml
t_end
, t_max
, and snapshot/statistics output times.I'm afraid this is not good. This'll need a rework. Fortunately, it's not much.
Have you tested whether this works correctly?