Preserve restart checkpoints and IB diagnostic history

[sbryngelson]

Summary

CFL restarts could overwrite their source checkpoint, chained runs could omit the last IB-force record and overwrite earlier force history, and CFL IB-marker saves could overwrite each other. This PR combines the three existing fixes so saved checkpoints and IB diagnostics retain their intended indices and run history.

Consolidation

This existing PR retains its original commits and discussion. Changes from #1950, #1961 are added as separate commits with original authors/messages and cherry-pick -x provenance. The complete source PR descriptions, verification records, and limitations are reproduced below. Their original verification claims are historical records, not fresh runs on this combined head.

The redundant merge of master on #1950 carried no merge-resolution changes; the combined branch merges current master directly and retains all four non-merge commits from #1950.

Verification of the combined branch

• The previous head of this PR remains an ancestor of the combined head.
• Every added/deleted line in the imported non-merge commits is preserved exactly.
• git diff --check passes excluding generated golden metadata; its original formatting is retained.
• ./mfc.sh precheck passes all seven gates: formatting, spelling, toolchain lint/tests, source lint, documentation references, parameter documentation, and example case validation.
• Full solver regressions and MPI reproductions were not repeated for this consolidation. Original records remain below, and CI must validate the combined head.

This consolidation was performed with OpenAI Codex.

Contribution Policy

We do not accept pull requests generated primarily by AI without genuine understanding or real-world usage context.

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.

---

Acknowledgement

• I confirm this PR meets the above expectations and reflects my own understanding and real-world context.

---

PR template credit: junegunn

Original PR documentation

#1960: CFL restarts: don't overwrite the restart checkpoint on the first step

Source: #1960

Original head: a50763e7cb51e04d5443287fc677e9a101bae43c

Complete original PR description

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.

	cfl_target 0.45	cfl_target 0.4
master: checkpoint n_start overwritten	n_start = 1, 2	n_start = 2
this PR: checkpoint n_start overwritten	none (1-4 unchanged)	none (1-4 unchanged)
fresh-run p_all, master vs this PR	byte-identical, indices 0-6	byte-identical, indices 0-6

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

We do not accept pull requests generated primarily by AI without genuine understanding or real-world usage context.

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.

---

Acknowledgement

• I confirm this PR meets the above expectations and reflects my own understanding and real-world context.

---

PR template credit: junegunn

#1950: IB force history: record each run's last step; don't overwrite on restart

Source: #1950

Original head: 4ac46752d967a0e620f27d02a7442585ce3c2870

Complete original PR description

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.

run	rows written	steps covered
straight 0→40, this branch	40	1..40
0→20 then restart 20→40, this branch	20 + 20 (ib_forces.dat, ib_forces_20.dat)	1..20, 21..40
0→20 then restart 20→40, master	19, then overwritten by 19	1..19, then only 21..39

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:

code	the 10 ib_forces tests
this branch	10/10 pass
master	10/10 fail: Variable count didn't match for D/ib_forces.dat (the missing last row)

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

We do not accept pull requests generated primarily by AI without genuine understanding or real-world usage context.

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.

---

Acknowledgement

• I confirm this PR meets the above expectations and reflects my own understanding and real-world context.

---

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

PR template credit: junegunn

#1961: IB markers: index each save by the save count under CFL time stepping

Source: #1961

Original head: 82e765240f06050cc2f2adc43d43487abfa4ae2a

Complete original PR description

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.

	Marker centroid error vs IB centre
master	saves 1–7 all show save 7's markers; errors of 4.24, 3.52, 2.80, 2.08 and 1.36 cells at saves 1–5
this PR	≤ 0.17 cells at every save

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.

Acknowledgement

• I confirm this PR meets the above expectations and reflects my own understanding and real-world context.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

Lines of Code

File	Lines	Diff
src/simulation/p_main.fpp	76	+3

Directory	Lines	Diff
simulation	28428	+3
total	47409	+3

[codecov]

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 61.80%. Comparing base (27cae2c) to head (503a8d5).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
src/simulation/p_main.fpp	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1960      +/-   ##
==========================================
+ Coverage   61.79%   61.80%   +0.01%     
==========================================
  Files          86       86              
  Lines       22773    22775       +2     
  Branches     3353     3354       +1     
==========================================
+ Hits        14073    14077       +4     
+ Misses       6211     6208       -3     
- Partials     2489     2490       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.