Repository navigation
ForceFreeStates - BUGFIX - Size the Galerkin elements below a singular surface with n*q1, as above it - #534
Merged
Merged
Conversation
…r surface with n*q1, as above it gal_make_grid! divided the element sizes next to a singular surface by |n q1| above it but by |q1| below the next one, so for n > 1 the elements below a surface were n times wider. Same fix as PrincetonUniversity/GPEC#326 for RDCON's gal.f. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
matt-pharr
self-requested a review
October 20, 2026 15:25
matt-pharr
marked this pull request as ready for review
October 20, 2026 15:25
matt-pharr
approved these changes
Oct 20, 2026
matt-pharr
left a comment
Collaborator
There was a problem hiding this comment.
Thank you for the fix Stuart! This one has been on my back burner for a long time. This matches the mentioned Fortran PR.
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.
Release note
gal_make_grid!sizes the resonant and extension elements next to a singular surface in units of the singular factor m − nq ≈ −n q1 (ψ − ψ_s). It divides by|n q1|above a surface (GalerkinGrid.jl:101) but by|q1|below the next one (:133, marked "faithful to gal.f"). For n > 1 the elements below a surface are therefore n times wider than those above it. This PR usesn q1on both sides, as PrincetonUniversity/GPEC#326 does for RDCON'sgal.f.To check that the symmetric choice is the right one, the size ratio of the elements below and above each surface was scanned on two DIII-D reconstructions at n = 2, with all element sizes (
gal_dx0,gal_dx1,gal_dx2) shrunk together atgal_nx512 (run on a branch whoseGalerkinGrid.jlis identical to develop's, with the below-side factor set at run time). Innermost surface (q = 1.5); Riccati on the same input is an independent method that does not use the Galerkin grid:singfac_min1e-4 / 3e-5 / 1e-5)On 174956, Δ′ converges as the elements shrink at every ratio, but to a different value for each ratio, and only ratio 1 matches Riccati (−0.7420 against −0.7418 to −0.7421). On the other surfaces the change is ≤ 0.2 %. For n = 1 the two expressions are the same, so nothing changes there.
The scan also shows a weakness of the Galerkin method that this PR doesn't fix: the innermost Δ′ depends on the size ratio of the elements on the two sides of a surface, and on 201883 it does not converge as the elements shrink, at any ratio.
Regression report
regress --refs 9fb32c9a6,fc330443f(develop vs this head after merging develop, all 19 cases; julia 1.12.6, 8 threads): every tracked quantity is unchanged except SLAYER γ_Hz indiiid_slayer_n1, which differs by 0.196 Hz (0.02 %) against a 0.1 Hz noise threshold. That case runsintegrator = "riccati", so the Galerkin grid this PR changes is never reached; its layer inputs all match exactly and only the AMR root extraction moved (Q_root by 1.2e-5, ω by 0.014 Hz), which is run-to-run noise.gal_resistive_peis N/A on both refs, as on develop. The Galerkin cases are n = 1, where the two expressions are the same; the n = 2 effect is in the tables above.🤖 Generated with Claude Code