EQUIL - BUGFIX - Sample knot midpoints so the round-trip check can see spline ringing - #443
Open
logan-nc wants to merge 1 commit into
Open
EQUIL - BUGFIX - Sample knot midpoints so the round-trip check can see spline ringing#443logan-nc wants to merge 1 commit into
logan-nc wants to merge 1 commit into
Conversation
…e spline ringing The efit_by_inversion round-trip residual (psi,theta) -> (R,Z) -> psi was sampled only at psi knots -- where the rzphi cubic splines interpolate exactly. Inter-knot ringing is invisible to that measurement by construction, so the check reported a clean edge while the reconstruction was unusable. Measured on the DIII-D-like deck at fixed mpsi=512, pushing psihigh toward 1: psihigh on-knot midpoint ratio et[1] 0.995 4.10e-06 5.02e-06 1.2 +0.80 0.999 3.57e-06 4.19e-06 1.2 +0.81 0.9999 1.14e-05 1.29e-03 113.1 +4.70 0.99999 2.65e-05 2.18e-02 823.2 -47707 The on-knot residual is flat across the whole range while the stability energies collapse by six orders of magnitude. The midpoint residual tracks the degradation, and their ratio is what actually separates the two regimes: dimensionless, so it needs no per-grid calibration, and good/bad sit two orders either side of 10. An absolute tolerance alone does not substitute. At psihigh=0.9999 the midpoint residual is 1.29e-3, under the existing 2e-3, so only the ratio catches that rung. Conversely the ratio alone would false-fire on a clean grid where both residuals are at the rounding floor, so it is consulted only once the midpoint residual clears ROUNDTRIP_TOL/20. Sampling widened from two fixed knots to every knot and midpoint in the outer ROUNDTRIP_EDGE_FRAC of the grid -- the region the check is named for and where the failure lives. The wider window matters: with only the two previously sampled knots and their neighbouring midpoints, the ratio at psihigh=0.99999 reads 1.0 and misses the worst rung entirely, because the ringing sits in an interval those samples do not touch. Verification: runtests_equil.jl passes 286/286, including a new false-positive control that a cleanly traced deck still reports at Info. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PSrf6JCViFfVzqzkQ66o6b
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
The
efit_by_inversionround-trip check measures(psi,theta) -> (R,Z) -> psionly at psi knots:https://github.com/OpenFUSIONToolkit/GPEC/blob/develop/src/Equilibrium/DirectEquilibriumByInversion.jl#L694-L716
The
rzphicubic splines interpolate exactly at their own knots. So the residual is near zerothere no matter how badly the splines behave in between — inter-knot ringing is invisible to this
measurement by construction, and the check reports a clean edge while the reconstruction is unusable.
Evidence
DIII-D-like deck, fixed
mpsi=512, pushingpsihightoward 1:et[1]The on-knot column is flat across the whole ladder while the stability energies collapse by six
orders of magnitude. The midpoint residual tracks the degradation.
Provenance, so nobody is misled by a single table: the three residual columns were re-measured
today through the shipped code path. The
et[1]column is from the earlier instrumented ladder thatran ForceFreeStates; today's script does not. The correlation claim — on-knot flat while the energies
collapse — was established on that earlier run, where both were measured together. The two runs also
used different sampling density, so the residual magnitudes are not directly comparable rung-by-rung
with that earlier table; the ordering and the separation are what reproduce.
The fix
Measure the residual at the knots and at the knot midpoints, and report both plus their ratio.
The ratio is the usable signal: dimensionless, so it needs no per-grid calibration, and good/bad sit
two orders of magnitude either side of a threshold of 10.
Two details that are load-bearing, both settled by measurement rather than taste:
psihigh=0.9999the midpoint residual is1.29e-3, under the existing 2e-3. Only the ratio catches that rung.
and their ratio is noise. So the ratio is consulted only once the midpoint residual clears
ROUNDTRIP_TOL/20.Sampling widened from two fixed knots to every knot and midpoint in the outer
ROUNDTRIP_EDGE_FRAC(0.9) of the grid — the region the check is named for and where the failure lives. This window
matters: I first tried the minimal version that keeps the two existing knots and adds only their
neighbouring midpoints. It reads ratio 1.0 at
psihigh=0.99999and misses the worst rungentirely, because the ringing lives in an interval those two samples don't touch. The band variant
catches every bad rung. (Widening further, to 0.75, pulls in coarser interior knots whose ordinary
interpolation error inflates the healthy baseline from 1.2 to 7.3 against a threshold of 10 — too
little margin, hence 0.9.)
A pre-existing defect this surfaces
On
test_data/CHEASE_test_data/EQDSK_COCOS_02atpsihigh=0.994, knot i=128 (psi=0.99385) traces to(R=3.32, Z=3.91) on a machine with
ro=7.04— a badly mis-located surface. The residual there is0.517.
This deck already warns on
develop: develop's outermost sample reads 0.048, well over 2e-3. Sothis is not introduced here — the warning just gets louder (0.517) and now names where the problem is.
I have deliberately not tried to fix the tracing defect in this PR. Flagging it for whoever owns
that path.
Separately,
psihigh=0.95and0.97on that same deck die in an unrelated pre-existingDomainError(sqrtof a negative) inequilibrium_global_parameters!atEquilibrium.jl:371.Also not touched here.
Verification
test/runtests_equil.jl— 286/286 pass, including a new false-positive control asserting acleanly traced deck still reports at Info with both residuals and the ratio in the message.
Ladder above re-measured through the shipped code path, not a replica.
Regression harness,
diiid_n1,--refs develop,localagainstdevelop @ 4e430f170:47 quantities, all unchanged.
Being straight about what that does and doesn't show:
diiid_n1useseq_type = "efit", so itdoes not execute the changed code at all. It confirms no leakage into other paths — the change is
log output only — but it is not coverage of this fix. No regression case currently uses
eq_type = "efit_by_inversion", which looks like a real gap worth closing separately; I'd ratherpropose that than bolt a new pinned case onto a small bugfix.
Scope
Equilibriumonly; no SLAYER/Tearing code touched. Split out of theFKR-widthwork as anindependent fix. Stacks cleanly alongside #442 (different files).
🤖 Generated with Claude Code
https://claude.ai/code/session_01PSrf6JCViFfVzqzkQ66o6b