Repository navigation
InnerLayer.GGJ - REFACTOR! - Solve the :ray BVP with the Galerkin assembly and use one parity convention - #536
Conversation
18c260b to
ff6feea
Compare
|
@logan-nc this is a cleanup of the GGJ side of InnerLayer. We revert back to using galerkin for the BVP part of the ray solver, and delete all the unused code. We also clean up exports. High line count but should be an easy review for that reason. |
|
@claude review this |
|
…l-ψ profiles opt-in
…erkin and ray solve paths
…e, tearing) and name parity by the displacement
…e behavior and paper pointers
…s states of the code
b877fae to
c8b68e4
Compare
… in the match profiles Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdRsGGg4YRG6E9fgLGbpdF
logan-nc
left a comment
There was a problem hiding this comment.
Nice cleanup. The single parity convention reads much better, and I checked that it holds everywhere I followed it: pen/bpen, the inner ξ and b profiles, the cin rows in MatchProblem, and all three solvers (interchange = Δ₊ = isol 1). The rotated weak form, the far-end IBP term and the DOF conversion agree with each other. Nothing still references the removed or un-exported names. I pushed one cosmetic commit (cf33eb8) that drops stray semicolons on the empty!(…) lines in MatchProblem.jl.
Four points for you to take or leave:
1. real_profiles=true takes bpen from the θ=0 solve, not the rotated one. solve_inner_profile(:ray) returns Ψ/Ξ from the θ=0 solve r0 (src/InnerLayer/GGJ/Ray.jl:449). MatchProblem reads pen from profΨ[1, :] (src/ForceFreeStates/Matching/MatchProblem.jl:150-151), so with profiles on, bpen comes from the very solve the PR says can fail at large |Q|. That contradicts the description's claim that bpen comes from the rotated solve. Suggested fix: overwrite row 1 (s = 0) of Ψ and Ξ with the rotated solve's values, which is about three lines, plus a test asserting that bpen is the same with real_profiles on and off.
2. certΔ checks only Δ₊ (Ray.jl:446). Δ₋ is the tearing channel bpen depends on, so it should be covered as well, e.g. certΔ = max(relerr(Δ[1]), relerr(Δ[2])).
3. The harness stamp b877faece no longer exists after the rebase, so the release-note check fails. I ran the harness at cf33eb8 against develop 84b7e48 and you're welcome to paste these results:
| Case | Result |
|---|---|
ggj_reference |
4 unchanged |
ggj_ray_q500i |
4 OK: Δ₊ diffs 5.1e-7 / 3.2e-7, Δ₋ ~1e-13 |
gal_resistive_diiid |
1 CHANGED: ‖inner-layer Δ‖ 1.474579e6, diff 4.7e-3 (0.00%); bpen diff 2.8e-8 |
gal_resistive_diiid_galinner |
11 unchanged |
gal_resistive_pe |
N/A on both refs |
Two caveats for whoever re-runs it:
- The harness cache is keyed only on (commit, case). This PR changes what
ggj_referencecomputes, so develop's cached rows are from the old problem. I used a fresh--db. - Run the harness from this branch's checkout. Develop's runner calls
glasser_wang_2020_eq55andq4_surface_benchmarkunqualified, and this PR un-exports them.
4. A design question. With the default model, deck runs that use gal_match_flag no longer write SingularSurfaces/Match/Inner/*, and no deck key turns them back on. You could add a ForceFreeStates control key (something like gal_inner_real_profiles), or keep it as a documented limitation. Either is fine with me, but it should be one or the other on purpose.
Minor things I'm not asking you to change:
- Passing
real_profilesto:galerkin/:shootingthrows an unsupported-kwarg error. That's fine since it's documented as ray-only. - The x=0 IBP term in
_parity_bcs_and_solve!is dead code, but it predates this PR. - The positional order of
InnerLayerResponseis already tracked in #538.
I did not review the InnerAsymptotics.jl deletions or the internals of _assemble_and_solve! line by line. For those I'm relying on your bitwise checks and the tests.
…ted solve when real profiles are requested
|
@logan-nc addressed your comments:
Regression harness is running, but the changes are final and ready for your approval. |
logan-nc
left a comment
There was a problem hiding this comment.
Thanks for the quick response!
|
Thanks @matt-pharr . One of my claudes had a few thoughts: "A post-merge read turned up a few things, none of which affect the parity convention: Δ± are paired consistently in every consumer, and
I'm happy to add the small fixes unless you'd rather, or neither if these are just superfluous? |
|
Thanks @d-burg, good catches. Yes please, go ahead with the fixes: the |
The :ray model returns no inner profiles by default since #536, so the cut-solution drift only shows with real-ψ profiles. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The :ray model returns no inner profiles by default since #536, so the cut-solution drift only shows with real-ψ profiles. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Follows #507 (merged).
Release note
:rayinner-layer Δ moves at ≲1e-6 relative (Hermite-cubic BVP in place of spectral collocation): Δ₊ by 9e-7 at q = 4, Q = 500i, and the matched DIII-D ‖Δ‖ by 2e-9, withbpenwithin 3e-8. Withreal_profiles=true,bpennow comes from the rotated solve as well.ggj_referencenow runs at a physical τ_A, τ_R (Q ≈ 0.22i), so its history restarts; the GGJ harness quantities are renameddelta_interchange_*/delta_tearing_*(same values as the olddelta_odd_*/delta_even_*, whose names were the wrong way round). (harness @ 128540b)solve_ray: thep,refine_tol,max_rounds,max_cellsandboundarykeywords are gone; the mesh is set bycfl,min_cellsandnq.RaySolveResultnow carries the Galerkin workspacewsand the Hermitebreaks.pick_xmax(usepick_smax),solve_inner_converged,cheblobatto,parity_rows, and the two-argumentsolve_inner_profile(params, γ).build_asymptotics,evaluate_asymptotics,InnerAsymptoticsCache,physical_ua_dua,RaySolveResult,q4_surface_benchmarkandglasser_wang_2020_eq55; useInnerLayer.GGJ.<name>.GGJModel()no longer writes theSingularSurfaces/Match/Inner/*profiles; passGGJModel(; real_profiles=true)or use:galerkinto get them.Δ,deltarandDelta_ralready were;RaySolveResult.Δrawand the Ψ/Ξ profile columns flip.The
:rayinner-layer backend now solves its boundary-value problem with the same Hermite-cubic Galerkin assembly as:galerkin, keeping the far-field march that makes it accurate at large |Q|; the collocation solver is gone. Matching takes the penetrated field from the rotated solve, so each surface is solved once, and every GGJ backend follows one documented parity convention.Regression report
Notes for reviewers
Why the collocation solver could go. An ablation (5400 solves: 3 surfaces × 100 Q values × 18 variants) found that
:ray's accuracy comes from its far-field treatment: series tolerance at the matching point, the inward Radau march with purges against a carried decaying frame, and Double64 at |Q| ≳ 100. Spectral order, residual-driven refinement, and collocation vs. Hermite Galerkin did not move Δ where the boundary data were good. The stock:galerkinbias at |Q| > 1 comes from matching where the series is only good to 1e-7, not from its interior.Commits
InnerLayer.GGJ - REFACTOR!Solve the:rayBVP with Galerkin.jl's building blocks and a point far-field condition; merge the duplicated real/complex asymptotic evaluators. Stock:galerkinand the evaluators are bitwise unchanged (checked). New:rayvs. old collocation: ≤3.5e-7; vs. a strict reference: worst 1e-6.InnerLayer.GGJ - API!bpencomes from the rotated solve (x = 0 lies on every contour), soMatchProblemruns one inner solve per surface; the θ = 0 real-ψ profiles become opt-in (real_profiles=true) with a θ = 0 vs θ = arg(Q)/4 certificate. Corrects the march comments (Radau IIA stability function, turning point).InnerLayer - REFACTOR!Removes dead code and duplicated solve paths in Galerkin.jl, Ray.jl and InnerAsymptotics.jl. Bitwise identical on 65 checked outputs.InnerLayer - API!One parity convention, from Pletzer, Bondeson & Dewar, J. Comput. Phys. 115, 530 (1994) and GWP2016 Eq. 34: parity is that of the displacement Ξ, Δ₊ even = interchange, Δ₋ odd = tearing, positional pairs ordered (Δ₊, Δ₋). Fixes reversed ± or odd/even labels in theInnerLayerResponsedocstring, docs tables, harness names, the convergence figure, and comments in CoupledFullMatch,run_slayer.jland SLAYER. Bitwise identical with parity columns mapped, including the full match path on the Solovev fixture. Follow-up for SLAYER and theInnerLayerResponsefield order: InnerLayer - Order InnerLayerResponse and SLAYER as (Δ₊, Δ₋) = (interchange, tearing) #538.InnerLayer - DOCSTrims the GGJ comments andinner_layer.mdto what the code does, with paper math replaced by equation pointers. Code unchanged (syntax trees compared).InnerLayer - DOCSCorrects paper citations after an audit against the PDFs: GWP2016 Appendix numbering (Q₀ A14, X₀ A15, S and X₀ = S^{−1/3} A16, Q = s/Q₀ A11), Wang et al. 2020 is Phys. Plasmas 27, 122503, the shooting method is Glasser, Jardin & Tesauro, Phys. Fluids 27, 1225 (1984) Sec. V rather than the DCON paper, the rescale exponent sign, the C_R sign in CoupledFullMatch, and GGJ 1975 Eqs. 16, 24 for D_I. Documents how:raydiffers from:galerkin(where and how the far-field condition is imposed, both radii computed per solve) and the measured small-|Q| behavior of:galerkin. Only code change:kl = mpert*(np+1) + 1 - 1→mpert*(np+1).InnerLayer - MINORJuliaFormatter over the files this PR changes; code unchanged (syntax trees compared).InnerLayer - API!Drops nineInnerAsymptoticsCachefields nothing reads (λ, Tinv, B, K2, Cmat, Dmat, Y0, Y0inv, Z) and an unusedK6array, and un-exports the GGJ internals and benchmark fixtures listed under Migration.delta_convergencestays exported as the documented error-bar tool. No numbers move.InnerLayer.GGJ - BUGFIX!(review) Withreal_profiles=truethe x = 0 row of Ψ, Ξ comes from the rotated solve, sobpenanddeltarno longer depend on the option;certΔcovers both Δ₊ and Δ₋. New MatchProblem-level tests:bpen/deltaridentical with profiles on and off,inner_*empty by default, and:galerkinvs:ray(real profiles) agreeing on the composite ξ and b at ψ_s, which catches a parity swap. The solution-profile figure plots the tearing solution again.Repo - BUGFIXThe figure tooling crashed on a clean tree (aSubStringcommit hash) and counted its own outputs as uncommitted changes, so figures could only be recorded as-dirty. Fixed; both inner-layer figures are regenerated from a clean commit (5d11aad3e).:galerkinat small |Q|. Against a tightened:rayreference,:galerkinis not uniformly safe at small |Q|: on the GW2020 Eq. 55 surface it errs by 7e-5 at Q = 0.1 and 3e-3 at Q = 0.3, and near arg Q = 157.5° by order unity at |Q| = 0.1 (0.4 on the q = 4 surface at |Q| = 0.3).:rayheld 2e-6 for |Q| from 0.01 to 1 on all three surfaces tested, at 3–60× the cost (12 ms to 0.24 s per solve). The docs now say so.Harness notes.
gal_resistive_peis N/A on both refs: its PE stage skips by design until the Galerkin free-boundary δW lands. The single CHANGED flag is ‖inner-layer Δ‖ ingal_resistive_diiidat 2e-9 relative.Known limits, not addressed here
certΔwarns when it does.real_profilesis a diagnostic and deliberately not a deck key, so deck runs withgal_match_flagomitInner/*.🤖 Generated with Claude Code
https://claude.ai/code/session_01K8p6NQZooWYJLqjayYjwcJ