Correct q sign-convention docs and enforce positive q (replaces #268) - #269
Merged
Conversation
… g-file The sign conventions reference claimed q is read directly from the EFIT g-file without sign manipulation. In the direct path the g-file q profile is read but unused: direct_run rebuilds q from a field-line integration using the sign-normalized psi map and |F|, so q is always positive for g-file input. Verified by running DCON on a DIII-D g-file and on a copy with all signed quantities negated: outputs (euler.bin, crit.bin, dcon.out, netcdf) are identical except timestamps. Also clarifies the COCOS section: sign conventions of the g-file are normalized away on read, only the per-radian flux units are required. Inverse equilibria keep the input q profile, including its sign.
…ip cruft The inverse path recomputes q by flux-surface integration in inverse_run (equil/inverse.f), like direct_run; the input q profile is unused, except the chease4 variant which copies it. Corrects the sign-conventions reference accordingly. Enforces positive q: guards newq0<0 in equil_read (the only source of negative q, previously undocumented and unvetted) and adds a positive-q check in inverse_chease4_run for files whose q column is itself negative. Removes the now-unreachable newq0==-1 / SIGN(one,newq0) sign-flip code from direct.f, inverse.f, and read_eq.f. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
2 participants
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.
This PR replaces #268 (which is from a fork the maintainers cannot push to). It contains all of #268's commits plus follow-up corrections from review. Once this is open, #268 can be closed.
Background
#268 set out to correct the safety-factor
qstatements in the sign-conventions reference. Review surfaced a factual error in #268's own text and a related code-hardening opportunity, addressed here.Documentation (
docs/sign_conventions.rst,CLAUDE.md)inverse_run,equil/inverse.f) recomputes q on each surface by flux-surface integration, exactly likedirect_run— the input file's q profile is read intosq_inbut unused. The text now reflects this.chease4format (inverse_chease4_run) takes q directly from the input file.newq0" wording — q is documented simply as always positive (this is not standard usage and shouldn't be advertised to new users).Code — enforce positive q (
equil/)GPEC requires positive q (positive
nn, positive resonantm; the mode-range and convention machinery assumeq > 0). Negative q was only reachable via undocumented, unvetted paths, now closed:equil.f: guard rejectingnewq0 < 0(the only knob that could force q negative), placed right after the namelist read.inverse.f(inverse_chease4_run): positive-q guardIF(MINVAL(sq%fs(:,4)) <= 0)for chease4 files whose q column is itself negative.direct.f,inverse.f(×2),read_eq.f: removed the now-unreachableIF(newq0==-1)newq0=-q0sentinel and*SIGN(one,newq0)sign-flip factor. If field/q reversal is ever wanted, it should be a clearly-named flag, not a magic negativenewq0.Verification
.flines ≤72 chars.newq0=-1→ rejected with a clear message;newq0=0→ q0=1.900;newq0=2.0→ q0=2.000, bit-identical to before the cruft removal (the removedSIGN(one,newq0)was+1for valid positivenewq0).docs/sign_conventions.rstparses under docutils (only the pre-existing Sphinx-only:doc:role message remains).🤖 Generated with Claude Code