Preserve restart checkpoints and IB diagnostic history #2059

Summary

In CFL mode (cfl_adap_dt / cfl_const_dt), a restart can overwrite its own restart checkpoint with the state one step later. @thierrydaoud spotted the symptom in #1926.

Mechanism

p_main.fpp saved when abs(mod(mytime, t_save)) < dt .or. mytime >= t_stop. A restart sets mytime = t_save*n_start exactly. After the first step, mod(n_start*t_save + dt, t_save) should equal dt, but because of round-off it comes out within an ulp of dt and falls below it about half the time. The save then fires and s_save_data writes index int(mytime/t_save) = n_start, so checkpoint n_start is replaced by the state after one step.

The cause is round-off, not a change in dt. The dt used in the check is the step's own dt, because s_compute_dt runs only at the start of s_perform_time_step.

Fix

Track the last saved index, starting at n_start. Save when int(mytime/t_save) moves past it, or at t_stop. This is the same expression s_save_data uses for the file index, so each index is written once and the restart checkpoint is never written again.

• Fresh runs save at the same steps as before: the first step at or past each k*t_save.
• The final save at t_stop is unchanged.
• If int(mytime/t_save) rounds down just after a crossing, the save comes one step late with the correct index. The old check would have written the previous index in that case.
• post_process only reads int(t_stop/t_save) and does not use the mod pattern. No other code uses mod(mytime, ...).

Reproduction (Frontier, CCE CPU, --no-mpi)

I modified examples/1D_sodshocktube/case.py to use cfl_adap_dt = T, t_save = 0.01, t_stop = 0.06, parallel_io = F, format = 1. I ran it fresh, then restarted it with n_start = 1..4, resetting p_all to the fresh run's output before each restart.

At one overwrite, the debug output showed mod(mytime, t_save) = 5.05147325735151587E-04 < dt = 5.05147325735151695E-04.

I also emulated the old and new save logic in float64 over 50k random dt sequences and t_save/t_stop combinations. The new logic never skipped or repeated a non-final index and never rewrote n_start. On fresh starts it saved at the same steps as the old logic. On restarts it differed only in the cases where the old logic rewrote n_start.

Regression test

New case Restart Roundtrip -> 1D -> cfl_adap_dt=T (25EB3D1F). For restart_check cases in CFL mode, run_restart runs the case straight, then restarts from each interior save index (1-4). After each restart it checks that p_all/*/<n_start>/ is byte-identical to what it was before.

The usual comparison of restarted output with the straight run is skipped for CFL cases. A CFL restart starts from a checkpoint stamped n_start*t_save rather than its true time, so its later states differ from the straight run.

t_save = 2^-8, so n_start*t_save is exact. On master this test fails here with Restart from n_start=1 rewrote its own restart checkpoint.

With the fix, the test is deterministic: no restart can write its own index. On master, whether the bug triggers depends on round-off, so a given compiler might not catch a regression. Checking four restarts makes a miss unlikely.

The golden file was generated with CCE 19 on Frontier.

Testing (Frontier login node, CCE CPU, --no-mpi)

• ./mfc.sh format, ./mfc.sh build --no-mpi -j 8
• ./mfc.sh test --no-mpi -j 8 -o <24 UUIDs>: every cfl_adap_dt / cfl_const_dt case and every restart_check case. 24 passed.
• ./mfc.sh test --no-mpi -j 8 -% 20: 142 passed, 0 failed.
• No existing goldens changed.

Tested on OLCF Frontier (CCE CPU, no MPI).

Contribution Policy

All contributions are expected to demonstrate:

• A clear understanding of the codebase
• Alignment with product direction
• Thoughtful reasoning behind changes
• Evidence of real-world usage or hands-on experience with the problem

If these expectations are not met, we would prefer to implement the changes ourselves rather than spend time reviewing low-effort submissions.

Description

Chaining a run with ib_force_wrt across restarts loses IB force history in two ways.

1. One step is missing at every restart. s_write_ib_force_history(t_step) runs at RK stage 1 of step N and records the force left by step N-1. The time loop in p_main exits as soon as t_step == t_step_stop, so the record for t_step_stop (the force after the run's last step) is never written. The next run deliberately skips its t_step_start, because at that point the force is still the one from before the run (the existing comment explains this). So no run ever writes that step. Our exports showed exactly one missing record at each restart step, e.g. steps 12735, 25470 and 38205 in a three-chunk run.
2. A restart wipes the history. s_open_ib_force_history deletes and recreates D/ib_forces.dat on every run. A restarted run therefore keeps only its own rows, and every earlier chunk's history is gone unless the user renamed the file in between.

Fix.

• p_main calls s_write_ib_force_history(t_step) once more after the time loop, which writes the run's last step. I did not write the resumed run's first step instead, because that would record the stale (zero) force the existing skip avoids.
• A resumed run writes D/ib_forces_<t_step_start>.dat (D/ib_forces_n<n_start>.dat with cfl_dt) instead of overwriting D/ib_forces.dat. A fresh run still writes D/ib_forces.dat. Within each file the documented layout is unchanged: row 0 is that run's first recorded step, there is no header, and offsets are fixed.
• docs/documentation/case.md describes the per-run files and the new last row.

Why not append into one file. That needs either absolute step-based rows, or a row base inferred from the existing file. Absolute rows leave NUL-filled holes whenever a run starts from a restart without the earlier history (the existing docs and comments avoid holes on purpose). An inferred base is silently misaligned after a chunk killed past its last restart dump. Separate files avoid both, and they still concatenate into the full history.

Impact on results and goldens

The physics is unchanged. Every run with ib_force_wrt now has one more row per body in D/ib_forces.dat. Ten goldens contain that file (135F548B, 49893269, 4BED9896, 5A22B45F, B317404C, C8AD6271, D6794F4C, E085CC5A, E5B66084, F200F862), and they are updated only on the D/ib_forces.dat line:

• I regenerated them on a Frontier CPU compute node (GNU 12.3 + Cray MPICH, Release).
• Using the harness's own packtol.compare at each test's tolerance (1e-10, Examples 1e-3), the regenerated values for every field, including the existing ib_forces rows, match the committed NVHPC 25.11 goldens.
• I then appended only the new final row(s) to each old ib_forces line. The old line is a string prefix of the new one, every other line and every golden-metadata.txt is untouched, and the change is one line per golden.
• The appended values come from the GNU run. If you would rather have them from your NVHPC lane, ./mfc.sh test --generate --only <those 10> reproduces them.

Verification

All runs were on a Frontier CPU compute node (GNU 12.3 + Cray MPICH, Release, 2 ranks). The case is a 2D circle moving at a prescribed velocity, with ib_force_wrt.

On this branch the two files concatenated match the straight run's ib_forces.dat: the same 40 times, and a maximum absolute difference of 2e-17 over all columns.

With the updated goldens, on the same node:

simulation compiles with CCE 19 (CPU) and GNU 12.3. Precheck passes, apart from two test_thermochem cases that fail on the Frontier login node because they compile with the system /usr/bin/gfortran (addressed by #1943).

Contribution Policy

All contributions are expected to demonstrate:

• A clear understanding of the codebase
• Alignment with product direction
• Thoughtful reasoning behind changes
• Evidence of real-world usage or hands-on experience with the problem

If these expectations are not met, we would prefer to implement the changes ourselves rather than spend time reviewing low-effort submissions.

We hit this in chunked production runs on Frontier; the fix was exercised on the small cases above.

Summary

With CFL-based time stepping (cfl_adap_dt or cfl_const_dt) and parallel_io = T (shared file, not file_per_process), post_process shows the wrong immersed-boundary markers for moving IBs: every save in a block of 100 shows the markers of that block's last save.

The simulation writes each save's ib_markers into restart_data/lustre_ib.dat at an offset set by the save number, computed as time_step/t_step_save, and post_process reads it back the same way. Under CFL stepping the caller already passes the save count (save_count = int(mytime/t_save)), and t_step_save keeps its default of −100. So k/(-100) is 0 for saves 1–99, −1 for 100–199, and so on: saves overwrite one slot. Fixed-dt runs are unaffected.

Change

• f_save_index(t_step, cfl_mode, t_step_save) in m_helper_basic returns t_step under CFL stepping, else t_step/t_step_save. The writer (s_write_parallel_ib_data) and the reader (s_read_ib_data_files) both use it, so the two cannot drift apart.
• The file layout is unchanged, so existing fixed-dt output still reads.

Verification

A 2D moving cylinder, 100×100, cfl_adap_dt = T, 8 saves, parallel_io = T, 2 ranks. For each save, the centroid of the post_process ib_markers field was compared with the IB centre in restart_data/ib_state_<save>.dat.

Found while rendering a 201-save shock–particle-curtain run with 38 moving reacting particles, where markers at save 40 sat 0.5 mm from the particles.

./mfc.sh test -o IBM: 61 passed. No regression test is added: the suite has no pattern for checking post_process output against IB state.

The bug was found and verified as described above.