GPEC - NEW FEATURE - Per-stage runtime records in gpec.h5 under Info/Runtimes

[logan-nc]

What and why

main_from_inputs timed every pipeline stage but only logged the durations, so an output file could not answer "where did the time go". This persists them under Info/Runtimes/<stage> (Float64 seconds, long_name + units="s" per the self-describing-HDF5 contract from #364), and teaches benchmarks/benchmark_git_branches.jl to report per-stage deltas averaged over its warm runs.

Stages, each recorded only when it actually runs: equilibrium, galerkin, force_free_states, tearing, perturbed_equilibrium, kinetic_forces, plus total.

The values nest rather than partition: galerkin is measured inside the force_free_states window and total spans the whole run, so the seven numbers do not sum. Both long_names say so, since that is the one thing a reader of the file cannot infer.

Timings are informational only — machine- and load-dependent. They are not regression quantities and should never be added to the harness.

Notes for review

• The completion banner at both exit paths now reuses total_dt, so the log line and the recorded value cannot disagree.
• _write_runtimes! guards on isfile (no-op when write_outputs_to_HDF5=false and nothing else wrote a file) and deletes an existing Info/Runtimes before writing, so reruns into an existing file stay idempotent. It targets the file this run actually produced: ctrl.HDF5_filename normally, or the SLAYER stage's pe_file when write_outputs_to_HDF5=false.
• Annotation reuses the existing Utilities.HDF5Annotations.annotate!; no new annotation path. test/runtests_h5_schema.jl enforces the metadata automatically — its walker fails if any Info/Runtimes/* dataset lacks long_name/units.
• The equilibrium-only early exit is untouched (no h5 exists there yet).
• In the benchmark tool the Info/Runtimes read is inside the warm-run loop, since each run overwrites gpec.h5. Refs predating the feature yield an empty dict and the table is skipped silently, so old-vs-new comparisons keep working.
• The stage token is tearing, not slayer: ALL - IMPROVEMENT - Self-describing HDF5 metadata (issue #226, PR B) #364 renamed the slayer/ group to Tearing/ under the "name the physics, not the algorithm" rule, and the other tokens already mirror their groups. The solver is named in the long_name instead.

Verification

Check	Result
runtests_h5_schema.jl	pass
runtests_fullruns.jl	pass (+2 runtime assertions)
runtests_rerun_from_h5.jl	pass (Info/Runtimes exempted from the bit-for-bit replay comparison)
Regression harness diiid_n1	48 unchanged, 0 changed — this diff only appends wall-clock numbers after all computation
Benchmark dry-check	read, missing-group fallback, missing-file fallback, shared-key averaging, total-last ordering, silent skip on empty/disjoint sides, zero-baseline Δ% guard

---

⚠️ NO PR IS EVER MERGED WITHOUT A THIRD-PARTY HUMAN REVIEWER'S APPROVAL — NON-NEGOTIABLE. ⚠️

🤖 Generated with Claude Code

[logan-nc]

Verification complete — post-merge

All checks re-run against the merged tree (eb944f9b):

Check	Result
runtests_h5_schema.jl	pass (14/14 + 6/6 group-name rule)
runtests_fullruns.jl	19/19 pass (17 before; +2 new runtime assertions)
Regression harness, diiid_n1	48 unchanged, 0 changed

Regression table

Regression Report: diiid_n1
Ref 1: 0ece9c4f  @ 0ece9c4f (2026-08-12)   <- branch parent
Ref 2: e695e890  @ e695e890 (2026-08-14)   <- feature commits
Summary: 48 unchanged

Zero movement, as expected — this change computes time() differences and appends to the h5 after all computation.

Note for anyone rerunning the harness on this branch

Two baseline traps produced misleading reports along the way; both are documented in the handoff doc:

1. Don't compare against origin/refactor/hdf5-metadata's tip. This branch's parent was ~60 commits behind it, so that comparison reports upstream physics changes (auto psi-grid Δ′ convergence fix, on-demand solution derivatives, coil psilim fix) — 38 quantities, nothing to do with this PR.
2. Don't compare a commit ref against local. Non-local refs run in a freshly instantiated harness worktree while local runs the working-tree environment, so the comparison includes environment drift. 0ece9c4f vs local shows 14 quantities flagged at 0.00% plus ODE steps (total) 1960 → 1968 — pure artifact. Comparing two commit refs through the same worktree path gives the clean zero above.

The case itself is deterministic: two --force runs of the same commit produced byte-identical values for all 49 quantities.

---

⚠️ STILL REQUIRES A THIRD-PARTY HUMAN REVIEWER'S APPROVAL BEFORE MERGE — NON-NEGOTIABLE. ⚠️

Also still required before merge: delete HANDOFF_h5_runtime_records.md.

[logan-nc]

Brought current with develop + four corrections

Branch updated. #364 merged this morning, so GitHub auto-retargeted this PR from refactor/hdf5-metadata to develop. Merged origin/develop in (641504fb) — content-free, since the merge base was already #364's head and develop's only new commit was that merge. The PR diff is now exactly this feature's 6 files, and with the base on develop the full Julia test suite runs in CI on this PR for the first time (test.yaml only triggers on PRs into main/develop, so it never ran while this was stacked).

Re-reviewed against the conventions as they finally landed. Four things needed fixing (40e550e3):

1. Info/Runtimes/slayer → tearing. ALL - IMPROVEMENT - Self-describing HDF5 metadata (issue #226, PR B) #364 renamed the slayer/ group to Tearing/ under the "name the physics, not the algorithm" rule; every other stage token already mirrors its group, so this was the straggler. SLAYER is now named in the long_name, where the solver belongs.

2. perturbed_equilibrium was recorded unconditionally. The push! sat outside the if "PerturbedEquilibrium" in keys(inputs) block, so a stability-only run stamped the stage at ~0.000 s for work that never happened — contradicting this PR's own "recorded only when they actually run" claim, and reachable in the existing fullruns examples.

3. _write_runtimes! could stamp a file this run never wrote — or skip the one it did. It hardcoded ctrl.HDF5_filename, but the SLAYER stage appends to pe_file, and PerturbedEquilibrium and KineticForces each carry their own write_outputs_to_HDF5 flag and filename. A run with ctrl.write_outputs_to_HDF5=false and pe_ctrl.write_outputs_to_HDF5=true produced a real output file that the guard ignored entirely, silently losing the timings. Each of the four write sites now records its path in a written_h5 local (last writer wins, matching where SLAYER already appends), and both exit paths stamp that file. This also replaces the write_outputs_to_HDF5 || slayer_result !== nothing proxy with the fact it was approximating.

4. print_stage_comparison divided by the branch-1 time with no zero guard, printing Inf%/NaN%. Item 2 made that reachable today; the column now shows an em dash when the baseline is zero.

Also: the galerkin and total long_names now state that the values nest rather than partition the run — galerkin is measured inside the force_free_states window and total spans everything, so the seven numbers do not sum. That is the one thing a reader of the file cannot infer. And docs/src/workflow.md's Info/ row now mentions Runtimes/, matching the row this PR already updated in hdf5-conventions.md.

The rest reviewed clean: the annotation table, the Info/Runtimes exemption in the replay comparison, the inside-the-warm-loop placement of the benchmark read, and the empty-dict fallback for refs predating the feature.

Local runtests_h5_schema.jl / runtests_rerun_from_h5.jl / runtests_fullruns.jl are running against the final tree now; I will post the results and correct anything they turn up.

Important

This PR requires a third-party human review approval of its current head before merge. No auto-merge label applied.

[logan-nc]

All checks green, and the suite ran against this branch in CI for the first time now that the base is develop:

Check	Result
runtests 1.11 - ubuntu-latest	pass
runtests 1.x - ubuntu-latest	pass
Tests	pass
Documentation / documenter/deploy	pass

Local, against the final tree: runtests_h5_schema.jl, runtests_rerun_from_h5.jl, and runtests_fullruns.jl all pass (19/19 on fullruns, including the two runtime assertions).

Direct check of the written files on two real runs, confirming the two behavioural fixes:

DIIID-like_SLAYER_example          -> [equilibrium, force_free_states, tearing, total]
Solovev_kinetic_calculated_example -> [equilibrium, force_free_states, total]

The SLAYER run records tearing, not slayer. The example with no [PerturbedEquilibrium] section records no perturbed_equilibrium key — before the gating fix it recorded one at ~0.000 s. Neither example configures Galerkin or KineticForces, so those tokens are correctly absent as well.

Important

Green CI is not review. This PR still requires a third-party human approval of its current head (40e550e3) before merge. No auto-merge label applied.

[logan-nc]

@priyanshlunia review & merge this in the next 24 hrs please. Just fix any minor issues yourself directly. Ping me if anything significant changes.

[priyanshlunia]

Context. The PR head (40e550e3) instrumented the monolithic main_from_inputs:
every stage timer, every HDF5 write site, and the _run_slayer_stage closure lived in one
function, so runtimes and written_h5 were plain locals visible to everything. Develop
then landed the staged-pipeline refactor (#393 line): main_from_inputs became a thin
orchestrator calling run_force_free_states, run_perturbed_equilibrium,
run_kinetic_forces, and run_slayer_stage, with the PE write moving inside the public
perturbed_equilibrium API. Two of the four write sites and two of the timers moved into
functions the orchestrator's locals can't reach.

The resolution taken: keep all bookkeeping decisions in main_from_inputs
and infer what the stage functions did, rather than changing their return values.
Two stage functions get an optional collector kwarg for the timers they own; everything
else is timed and tracked at the call sites. Merge commit: 30529ff7.

The three mechanisms

1. Call-site timing (equilibrium, force_free_states, perturbed_equilibrium, kinetic_forces, total)

main_from_inputs wraps each stage call in a time() window and pushes the record itself.
Semantics match the PR head: perturbed_equilibrium is recorded only when the deck has a
[PerturbedEquilibrium] section (the stage function logs its window unconditionally, but a
section-less run must not stamp a ~0 s entry), and kinetic_forces only when
[KineticForces] is present.

2. Collector kwarg (galerkin, tearing)

Two timers measure something inside a stage function, so the call-site window can't
reproduce them:

• run_force_free_states(...; runtimes=nothing) — pushes "galerkin" => dt around the
  Galerkin solve. Note the semantic shift inherited from develop: galerkin is now an
  exclusive integrator choice, not a gal_flag add-on, but it still nests inside the
  force_free_states window, so the annotation wording holds.
• run_slayer_stage(...; runtimes=nothing) — pushes "tearing" => dt around
  Runner.run_slayer only (excluding the HDF5 append), preserving the PR head's
  solver-only semantics.

Both default to nothing and no-op when absent, so the scripting API path
(solve(::EulerLagrangeProblem, ...), which also calls run_force_free_states) is
completely untouched.

3. Write-site inference (written_h5)

The PR head assigned written_h5 directly at each of the four write sites. Two of those
sites are now out of reach, so the orchestrator mirrors their write gates instead
(src/GeneralizedPerturbedEquilibrium.jl:244–332):

Stage	Where the write happens now	Inference in main_from_inputs
ForceFreeStates	still in the orchestrator	direct assignment inside the existing if ctrl.write_outputs_to_HDF5 block (unchanged)
PerturbedEquilibrium	inside public perturbed_equilibrium, gated on pe_ctrl.write_outputs_to_HDF5 (default true) with filename fallback output_filename == "" → ctrl.HDF5_filename	section present && get(inputs["PerturbedEquilibrium"], "write_outputs_to_HDF5", true) → written_h5 = pe_file; the pe_file derivation (which already mirrored the filename fallback for SLAYER) was hoisted above the PE call
KineticForces	inside run_kinetic_forces, gated on section && pe_state !== nothing && kf_ctrl.write_outputs_to_HDF5	same three conditions, all visible in the orchestrator (kf_ctrl is in scope)
SLAYER	inside run_slayer_stage's guarded try	slayer_result !== nothing ⟹ the Tearing/ append succeeded → written_h5 = pe_file (or ctrl.HDF5_filename on the force_termination path)

"Last writer wins" ordering and the two stamp points (the force_termination early return
and the final banner) are unchanged from the PR head, as are _write_runtimes!,
RUNTIME_H5_ANNOTATIONS, the tests, the benchmark-script changes, and the docs rows.

What changed relative to the PR head, in one list

1. Removed the _run_slayer_stage closure instrumentation (the closure itself is gone
   on develop) → replaced by the runtimes kwarg on the top-level run_slayer_stage.
2. Moved the galerkin timer into run_force_free_states via the same kwarg pattern.
3. Moved PE/KF timing from inside the (former) inline blocks to call-site windows —
   numerically equivalent (same window + function-call overhead).
4. Replaced direct written_h5 assignments at the PE and KF write sites with the
   gate-mirroring inference above; SLAYER's assignment became result-based inference.
5. Reverted a bad auto-merge: git had spliced the PR's push!(runtimes, "perturbed_equilibrium" => ...) into run_perturbed_equilibrium, where runtimes
   doesn't exist — it parsed fine and would have thrown UndefVarError on every PE run.

Trade-off and review focus

This implementation keeps the diff minimal and every stage function's signature/return contract
intact (only two additive kwargs), at the cost of duplicating three write-gate
conditions in the orchestrator. If a default flips (e.g.
PerturbedEquilibriumControl.write_outputs_to_HDF5) or a write gate gains a condition, the
mirror in main_from_inputs must follow — each mirror carries a one-line comment naming
what it mirrors. These three conditions are the right review focus; the alternative
(stage functions return the filename they wrote) is drift-proof but touches
internal signatures — easy to switch to later if the mirrors prove fragile.

Verification: 99 tracked harness quantities across 5 runnable cases, zero moved;
19/19 fullruns tests plus schema and replay tests pass; Info/Runtimes confirmed in real
outputs including the tearing record.

[priyanshlunia]

@logan-nc pinging you since this was a relatively significant edit.