Repository navigation
ForceFreeStates - BUGFIX! - ⚠️ Keep both crossings of a rational surface on reverse-shear profiles - #492
Draft
d-burg wants to merge 10 commits into
Draft
ForceFreeStates - BUGFIX! - ⚠️ Keep both crossings of a rational surface on reverse-shear profiles#492d-burg wants to merge 10 commits into
d-burg wants to merge 10 commits into
Conversation
… on reverse-shear profiles The singular-surface finder identified a surface by its q value, so the two crossings of q = m/n on either side of an off-axis q minimum were merged into one entry carrying m = [m, m]: the multi-n merge path intended for (2,1)/(4,2) collisions. The outer crossing then had no chunk boundary, no singfac_min standoff and no ideal jump, the sweep crawled through an unregularized 1/(m - nq) (hundreds of thousands of steps), the Δ′ BVP was skipped as a multi-resonance surface, and the free-boundary energy was wrong by a factor of two against STRIDE on DIII-D 153072 (q₀ = 2.44, q_min = 1.86). Identify surfaces by ψ instead, in both sing_find! and the auto-grid knot list, so a genuine multi-(m, n) collision still merges while distinct crossings stay distinct surfaces with their own ψ and q′ sign. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…me unresolvable crossings Deduplicate rational-surface roots on both q and psi, so two different rationals can never merge even where steep q puts them within tolerance in psi. Replace the chunk-bounds assert with an error naming the surface when its singfac_min standoff overruns, which happens for crossings near a q extremum. Add a unit test that swaps a synthetic reverse-shear q into the Solovev fixture and checks both q=2 crossings, their opposite-sign shear, and that (2,1) and (4,2) still merge on each. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…or and document the surface merge rule The chunk-bounds error no longer asserts a q extremum or recommends reducing singfac_min; it reports the surface, q', the standoff and the chunk start. sing_find! documents the equal-ratio merge and reverse-shear split in its docstring and drops the Fortran citation. Refresh a stale dedupe comment in bracket_mandatory_nodes and tidy the reverse-shear test's isapprox call. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…crossing standoffs Two crossings of q = 2 on an analytic parabola resolve when 2ε > singfac_min and raise the named error when their standoffs overlap; a tangent surface (q' = 0) also raises it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sed-shear profiles When qmin < qlow < q0 the q < qlow region is a ring around q_min rather than a core, and sing_min! (outer leg) and initialize_el_at_axis! (axis) disagree about the lower integration bound. check_qlow_domain raises a named error at domain setup saying what was detected and to set qlow <= qmin or qlow >= q0. Covered on the analytic reverse-shear fixture in runtests_sing.jl. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…kets overlap bracket_mandatory_nodes now errors when c2 - w2 <= c1 + w1 for adjacent brackets, or when a half-width is non-finite (tangent surface), instead of interleaving bracket knots and leaving a surface off-center or sharing a spline interval. The collapse_atol dedupe is kept as a safety net. Geometric condition only; no new tolerance. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… with an overlapping standoff The kinetic chunking branch still skips a kinetic singular surface whose crossing standoff reaches back past the chunk start, but now warns with its m/n, psi, q' and standoff. No result changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ear-double-rational # Conflicts: # src/ForceFreeStates/Surfaces/Finding.jl # src/GeneralizedPerturbedEquilibrium.jl
github-actions
Bot
added
bugfix
Contributor
|
This pull request is missing a reviewer. If you are not ready to name them, mark this pull request as a draft. |
d-burg
changed the title
Collaborator
|
Just attaching Matt here since this seems like a big deal |
This branch has not been deployed
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.
Release note
qlowbetween q_min and q0 on a reversed-shear profile, and rational surfaces whose grid brackets overlap, now raise named errors.On a reversed-shear profile, the two crossings of q = m/n on either side of an off-axis q minimum were merged into one surface. The outer crossing then had no chunk boundary, no standoff and no ideal jump, and the free-boundary energy was wrong by a factor of two against STRIDE on a DIII-D reversed-shear case. Surfaces are now identified by ψ, so distinct crossings stay distinct while genuine multi-(m, n) collisions still merge.
Regression report
regress --refs cb18e386e,8e9733f0a --force(all cases) on feynman (SLURM, 8 threads; current develop vs head after merging develop, both fresh, manifest pinned). Every quantity is unchanged except the SLAYER γ line (0.01 %), which is develop's unseeded-triangulation noise. No shipped deck trips the two new errors.ggj_ray_q500isolovev_kinetic_nuzerodiiid_error_fielddiiid_slayer_n1gal_resistive_pesolovev_kinetic_calculatedgal_resistive_diiidsolovev_n1ggj_referencesolovev_multi_ndiiid_n1solovev_kinetic_ntvdiiid_n1_riccatisolovev_kinetic_multiiondiiid_multi_nefit_fixedbdy_separatrixgal_resistive_peextracts nothing on either ref; that is pre-existing on develop. Tests at8e9733f0a:runtests_sing.jl87/87,runtests_eulerlagrange.jl114/114 and 26/26,runtests_grid_refinement.jl62/62.Notes for reviewers
8e9733f0a). The two conflicts were additions on both sides at the same place:check_qlow_domainfrom this branch andremove_singular_surfs!from develop inFinding.jl, and both names on one import line. Both are kept; theqlowcheck runs before surface finding and develop's filter after it.NO MERGE WITHOUT HUMAN REVIEW. This PR is a draft and needs a named human reviewer before it can be considered for merge.
🤖 Generated with Claude Code