Repository navigation
Fixed-dt runs: keep the clock on t_step*dt so restarts continue bitwise - #1951
Conversation
With a fixed dt, a restart starts its clock at t_step*dt (p_main) and evaluates prescribed IB kinematics there (s_ibm_setup), but an uninterrupted run accumulates mytime = mytime + dt, which drifts (1290 ulps by step 27000 at dt = 6.17e-4). Stage times were likewise mytime + dt. And the last step of every run trimmed dt to finaltime - mytime, so a chunk's final step used a dt off by that drift. A restarted run therefore never continued the uninterrupted one exactly; near-tie immersed-boundary cells amplify such round-off to visible local differences. For fixed dt, advance mytime as (t_step + 1)*dt, form the IB stage times from t_step the same way, and drop the fixed-dt trim (the step count already lands on t_step_stop). cfl_dt runs are unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
3 open findings
What changed in this PR
This PR makes fixed-dt runs bitwise reproducible across restarts by aligning the simulation clock and immersed-boundary (IB) stage-time evaluation to exact t_step*dt, and by removing last-step dt trimming that depended on accumulated floating-point drift.
Changes:
- Update fixed-
dtclock advancement tomytime = (t_step + 1)*dt(keepcfl_dtbehavior unchanged). - Compute IB Runge–Kutta stage times from
(t_step + 1)*dt/(t_step + 0.5)*dtfor fixed-dt, and passt_stepinto propagation routine. - Remove
finaltimeand the fixed-dt“last-step trim” logic.
| File | Description |
|---|---|
| src/simulation/p_main.fpp | Removes finaltime initialization tied to t_step_stop*dt. |
| src/simulation/m_time_steppers.fpp | Makes IB stage times restart-consistent for fixed-dt by using t_step*dt forms and threads t_step into IB propagation. |
| src/simulation/m_start_up.fpp | Removes fixed-dt last-step dt trim and advances mytime via (t_step + 1)*dt for fixed-dt. |
| src/simulation/m_global_parameters.fpp | Removes global finaltime parameter. |
🧠 Review effort: Lite
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Lines of Code
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1951 +/- ##
=======================================
Coverage 62.64% 62.65%
=======================================
Files 86 86
Lines 22425 22437 +12
Branches 3325 3327 +2
=======================================
+ Hits 14048 14057 +9
Misses 6119 6119
- Partials 2258 2261 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|


Description
With a fixed
dt, a run restarted from step N does not continue the uninterrupted run exactly. For a flapping-wing case (patch_ib%kin_model = 1) on Frontier, a restart from step 27000 differed from the uninterrupted run by 5.9e-4 inrho*uafter 270 steps. The difference was deterministic, independent of rank count, and confined to a few cells at the wing root. A pitch-up case (kin_model = 2) happened to agree to 1e-11.Root cause. Three things make the restarted run and the uninterrupted run use slightly different times:
src/simulation/m_start_up.fpp:656). The uninterrupted run advancesmytime = mytime + dt, a running sum that drifts fromt_step*dt. A restart starts the clock att_step*dt(p_main.fpp:55) and evaluates prescribed IB kinematics att_step_start*dt(m_ibm.fpp:101). Withdt = 6.17e-4the sum is 4.6e-12 (1290 ulps) ahead at step 27000. So for the whole restarted run, the kinematics, inflow ramps and forcing are all evaluated at times offset by that amount.m_time_steppers.fpp:845-846). Each RK stage evaluates the prescribed kinematics atmytime + dt(ormytime + dt/2). That is not bitwise(t_step + 1)*dt, which is what a restart evaluates.m_start_up.fpp:610-613). The last step of every fixed-dt run setsdt = finaltime - mytime. That changes the step's dt by the accumulated drift, a step the uninterrupted run takes with the true dt. This alone makes even a restart with no IB differ, at 1e-15.On their own these are round-off. A moving IB amplifies them: whether a cell is inside the body, or which cell an image point lands in, is a near-tie that a sub-ulp shift can flip. The pitch-ramp test comment in
cases.pydocuments a 5e-13 perturbation moving the field by 5e-3. Whether a restart shows a visible difference therefore depends on whether such a near-tie cell exists near the body at that moment, not on the kinematic model; the flapping and pitch-ramp models use the same code path.All other state is restored exactly:
kin_model > 0,s_ibm_setupre-evaluates the kinematics analytically att_init, overridingib_state_<step>.dat(which is fullreal(wp)anyway).ib_offset_<step>.datis writtenES24.16, which round-trips doubles exactly.Fix. For a fixed
dtonly (cfl_dtruns are unchanged):mytime = (t_step + 1)*dt;(t_step + 1)*dtand(t_step + 0.5)*dt;t_step_stop(the now-unusedfinaltimeis removed).Is the old behaviour a bias? No. It is a one-time round-off perturbation per restart, amplified locally; it is not a systematic drift. In the production case the integrated force differed by 3e-6 relative over the 270 steps. The cost is that restarted runs are not bit-reproducible continuations, which makes restart checks hard to interpret.
Verification
These runs were on a Frontier CPU compute node (GNU 12.3 + Cray MPICH, Release, 1 rank), with
dt = 5e-4and serial restart I/O. Each compares a straight 0→40 run with a 0→20 run restarted 20→40, bit for bit at step 40:masterWith only the clock change, and before dropping the trim, the IB cases still differed; the no-IB and static-IB cases traced the remainder to the last-step trim. Saving every step versus every 20 versus only at 40 makes no difference, so saving itself perturbs nothing.
Regression: 114 tests that use time-dependent features (all IBM tests including moving and prescribed kinematics, acoustic sources, synthetic turbulence, body forces, restart roundtrips) pass against their existing goldens. Results change only at round-off, so I did not change any goldens.
simulationcompiles with CCE 19 (CPU) and GNU 12.3. Precheck passes, apart from twotest_thermochemcases that fail on the Frontier login node because they compile with the system/usr/bin/gfortran(addressed by #1943).Not addressed. With
cfl_dt, a restart setsmytime = t_save*n_start, while the saved state is at the actualmytime, which can differ from that by up to onedt. This is a separate and larger restart inconsistency.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:
If these expectations are not met, we would prefer to implement the changes ourselves rather than spend time reviewing low-effort submissions.
Acknowledgement
This PR was prepared with the assistance of an AI tool (Claude Code). We found this in restart tests of production runs on Frontier; the fix was exercised on the small cases above.
PR template credit: junegunn