gpec.h5 schema: follow-ups from the #364 standardization audit

[logan-nc]

Non-blocking items surfaced by the two independent reviews run against #364 (a whole-schema standardization audit and a devoted physics units review). Everything blocking was fixed in #364 itself; these were deliberately left out to keep that PR reviewable and because several are data-layout or out-of-scope-file changes.

Re-verified against develop at b3abe074 (#363 and #364 both merged). All items below still reproduce; the wording of items 1, 2 and 4 has been corrected where the original text was inaccurate.

Schema / data layout

1. ξ array axis order differs between integrator backends.
ForceFreeStates/Solutions/ForwardIntegration/xi_psi is (mode, solution, psi); ForceFreeStates/Solutions/GalerkinIntegration/Solution/xi_psi is (mode, psi, solution) — the same physical quantity, transposed. #364 unified the names (both are xi_psi/dxi_psidpsi now), each dims attribute is honest about its own order, and the Galerkin long_name now carries an explicit warning ("note the psi/solution axis order differs from ForwardIntegration"), so the trap is at least signposted in the file. Cross-backend tooling that reads one and then the other still silently gets a transpose. Unifying requires a permutedims at write plus updates to benchmarks/verify_gal_{solution,ideal}.jl and friends — a data change, not a naming one, so it wants its own PR and a regression run.

2. Tearing/PerSurface/ cannot be plotted against ψ.
PerSurface/ already writes m, n and rs per surface, so the resonant q = m/n is derivable — but rational_psi is absent, and Roots/ and LayerWidths/ carry only the opaque rational_index. Attach paths resolve relative to the annotated group, so cross-group dimension-scale attachment to SingularSurfaces/rational_psi is not mechanically possible — the fix is writing rational_psi (and rational_q for symmetry with the rest of the schema) directly into Tearing/PerSurface/. The file's own (now stale) header comment suggests that was the original intent. Would make Tearing results natively plottable vs ψ or q like SingularCoupling/ already is.

3. PE Response/ writes empty placeholder arrays where sibling code skips.
src/PerturbedEquilibrium/Utils.jl:143-148 unconditionally writes xi_psi, b_psi_area_weighted, Jb_theta, Jb_zeta, b_n, xi_n as ComplexF64[] when not computed (so a rank-1 empty array carries a rank-2 dims attribute), while the surrounding function and the main writer both use !isempty(...) && guards or zero-extent sentinels. Pick one pattern file-wide.

4. Optional completeness. EulerLagrangeMatrices/Kinetic/{D,E,H,F} are shared unchanged from Ideal/ and never re-emitted — table entries were trimmed accordingly, but writing the copies for symmetry is an option. (Equilibrium/b_norm was also listed here; it has since been folded into #389, which owns the decision to compute the field or delete it. Do not fix it in both places.)

5. Vocabulary watch item. dims uses both solution (arbitrary-amplitude basis index, e.g. ForwardIntegration/xi_psi) and eigenmode (normalized generalized eigenvector index, e.g. FreeBoundaryStability/eigenmode_energies). These look like genuinely distinct concepts, but a schema owner should confirm rather than leave it implicit. Similarly Tearing/PerSurface/lu could be S, the literature symbol for the Lundquist number — its long_name already reads "Lundquist number S per surface", so only the leaf name is out of step. Low value, listed for completeness.

Documentation defects found during the units review

6. Metric coefficient labels are dimensionally inverted. src/ForceFreeStates/Fourfit.jl:35-40 (the docstring listing "1. g^ψψ · J … 6. g^ψθ · J", and the MetricData.fs field docs) describes metric.fs[1..6] as g^ij·J. The code computes g_ij/J: v[i,:] = e_i/J (e.g. v[3,3] = 2π·R/jac, which is |e_ζ|/J, not ∇ζ = 1/(2πR)), then fs[k] = dot(v_i, v_j) * jac. Verified independently. Numbers are unaffected — only the label is wrong — but it will mislead the next person auditing units. The same wording is repeated in src/PerturbedEquilibrium/FieldReconstruction.jl:611, and a nearby block comments its covariant basis vectors as "Contravariant basis vectors WITHOUT /jac". Highest value-per-effort item here: a wrong dimensional label inside a physics kernel.

7. Two smaller comment corrections in FieldReconstruction.jl: the # b^ψ (not J-weighted) (:797) / # b^ψ (no Jacobian) (:191) comments describe a quantity that is J·b^ψ (χ₁ supplies the weight; what is absent is the jmat convolution); and the area = <J·|∇ψ|>_θ (Hamada: J is const over θ) caveat at :133 is unnecessary — the (J·b^ψ)_m/A = b̄_m identity is exact in any SFL working coordinate.

8. Stale agent memory. .claude/agent-memory/fortran-physics-reviewer/known_issues_perteq.md:7-9 states xss is a zero placeholder; it is now solved as -A⁻¹(B·xmp1 + C·xsp) in compute_clebsch_displacements. The MEMORY.md index line says the same.

Suggested split

Items 1–3 are the substantive ones and each wants its own PR with a regression run. Items 6–8 are documentation/comment-only and could land together in a single cheap pass. Items 4–5 are judgement calls for whoever owns the schema.

Context: #226 (schema overhaul), #363 and #364 (merged), #389 (equilibrium parameter cleanup, owns b_norm).

[logan-nc]

Re-verified every item against develop at b3abe074 (both #363 and #364 merged). Nothing here has been fixed in the interim — the commits that landed after the audit ran were the Clebsch rename, the Equilibrium scalar overhaul and the psi_boundary long_name fix, none of which touch these files.

Three items were reworded because the original text was inaccurate:

• Item 1 understated the current state: the Galerkin xi_psi long_name now carries an explicit "note the psi/solution axis order differs from ForwardIntegration" warning, so the transpose trap is signposted in the file even though it is not fixed.
• Item 2 overstated it: PerSurface/ already writes m, n and rs, so the resonant q = m/n is derivable today. Only rational_psi is genuinely missing (and Roots//LayerWidths/ still carry the bare index).
• Item 4's b_norm half is now owned by Equilibrium parameters: compute the stubbed flux bookkeeping fields, and collapse the duplicate psi-on-axis variables #389, which decides whether to compute the field or delete it. Removed from here so it does not get fixed twice.

Also added line references throughout and a suggested split: items 1–3 each want their own PR with a regression run, items 6–8 are comment-only and could land as one cheap pass, items 4–5 are schema-owner judgement calls. Item 6 is the highest value per unit effort — a dimensionally inverted label (g^ij·J documented where the code computes g_ij/J) sitting in a physics kernel's docstring.

[logan-nc]

Assigning to @matt-pharr.

Re-verified against develop at b3abe074 — all eight items still reproduce, and the body now carries line references so nothing needs re-hunting.

Where I would start, if it helps: item 6 is the highest value per unit effort. src/ForceFreeStates/Fourfit.jl:35-40 documents metric.fs[1..6] as g^ij·J, but the code computes g_ij/J — the numbers are correct, only the label is inverted, and it sits in a physics kernel where it will mislead the next units audit. Items 7 and 8 are comment-only corrections in the same neighbourhood and could ride along in one cheap pass.

Items 1–3 are the substantive ones and each wants its own PR plus a regression run; item 1 in particular is a data-layout change (permutedims at write) that also touches benchmarks/verify_gal_{solution,ideal}.jl. Items 4–5 are schema-owner judgement calls rather than defects.

One coordination note: Equilibrium/b_norm was originally listed under item 4 here but is now owned by #389 (@jhalpern30), which decides whether to compute the field or delete it outright. It has been removed from this issue so it does not get fixed in both places.

[matt-pharr]

Coordination note from the #393 side (ForceFreeStates integrator abstraction): we are incorporating items 1 and 2 into #393 as its final commit, and leaving the rest here for follow-ups.

• Item 1 (ξ axis order) — in its minimal form: per-formalism groups stay, but the Galerkin writer transposes at write so every Solutions/*/xi_* dataset shares the ForwardIntegration axis order (dims attributes and the gal readers/benchmarks updated to match). Rationale for pulling it into FFS - REFACTOR - Publish solves as ForceFreeStatesResult; staged main; solve API (refactor plan interface PR) #393: that PR's whole contract is "same physics ⇒ same layout regardless of integrator" — it already unified the Δ′/PEST-3 payload into one DeltaPrimeData on shared SingularSurfaces/ paths — and doing the ξ half there means the gal regression cases re-baseline once instead of twice. The full unification (writing gal ξ through the shared SolutionProfiles interchange type that FFS - REFACTOR - Publish solves as ForceFreeStatesResult; staged main; solve API (refactor plan interface PR) #393 introduces) stays future work; SolutionProfiles is the natural vehicle for it when someone picks it up.
• Item 2 (Tearing/PerSurface/rational_psi/rational_q) — FFS - REFACTOR - Publish solves as ForceFreeStatesResult; staged main; solve API (refactor plan interface PR) #393 enables Galerkin-fed SLAYER, and gal's Δ′ indexes the in-domain subset of the singular surfaces, so tearing output must identify its own surfaces to be unambiguous. That makes this item practically owed by FFS - REFACTOR - Publish solves as ForceFreeStatesResult; staged main; solve API (refactor plan interface PR) #393 rather than optional.

Staying here on the issue: item 3 (we'd suggest deciding the empty-vs-sentinel pattern once, aligned with #368's zero-extent sentinels, after both PRs land), items 4–5 (schema-owner calls), and items 6–8 (fit a comment-audit pass in the #354 style).

🤖 Generated with Claude Code

[d-burg]

Adding a schema item from the #393 Δ′ unification, since @matt-pharr asked there whether Δ′ should
move into the per-integrator folders and this issue is the right home for it.

Recommendation: keep Δ′ where it is, and record the formalism in one field.

Against moving it: this issue's parent principle consolidates per-rational-surface results under
SingularSurfaces/ regardless of which algorithm produced them, with provenance in the subgroup
name. Δ′ is a property of the surface, not of the solver, and SLAYER/GGJ/Tearing all want one
canonical path rather than dispatching on backend. Moving it also re-breaks the harness cases #393
just re-pointed.

For the single field: #393 removed gal_flag and made "galerkin" a standalone integrator, so
exactly one formalism now runs per invocation and a global field is unambiguous. That was not true
before — gal_flag = true layered gal onto riccati and both Δ′s coexisted in one file. Info/ already
holds run metadata (git_version, mpert, psilim), so Info/integrator fits the existing pattern.

One conventions wrinkle worth settling explicitly. "Inputs live only under Input/" forbids
echoing control-struct values into output groups, and read naively, Info/integrator looks exactly
like that. The distinction is that a record of which code path executed is derived output — an
observation about the run — not an echo of the requested configuration. Worth one sentence in
hdf5-conventions.md, otherwise the next schema audit flags it, which is how this came up in the
first place.

Why it matters beyond tidiness. After #393 the Galerkin and BVP Δ′ share
SingularSurfaces/Delta_prime_matrix and are both in the PEST-3 convention, so nothing in the file
distinguishes them. Flip a deck's integrator and a golden value pinned from one formalism is
silently compared against the other. In #397 I pin the deck's declared integrator out of
Input/gpec_toml_raw as a stopgap, but that only works for TOML-driven runs — solve(prob, Galerkin())
writes no input blob, so an API-produced file has no recoverable provenance at all. A written
Info/integrator fixes both, and records what actually ran rather than what was asked for; the
harness pin would then read it instead.

Happy to take this one if nobody else has it — it is small, and #397 wants it.