InnerLayer.SLAYER - TEST - Pin the reverse-shear invariance of the layer magnitudes - #432
Merged
Merged
Conversation
…yer magnitudes The shear-magnitude fix has no test coverage: every SLAYER testset builds its parameters at sval_r = 1.0, and no shipped deck has a reverse-shear rational surface, so a regression would be silent. Assert that a negative-shear surface reproduces its positive-shear mirror bit-identically across every normalized layer quantity, that sval_r keeps its sign in the diagnostic, and that the Lundquist number stays positive so the S^(1/3) that used to raise a DomainError evaluates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
logan-nc
approved these changes
Aug 28, 2026
…invariants Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
d-burg
requested review from
ebursch
and removed request for
ebursch and
matt-pharr
August 28, 2026 20:09
d-burg
marked this pull request as ready for review
August 28, 2026 20:57
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
src/.#431 made the SLAYER layer timescales and widths use
|s|so a reverse-shear rational surface produces a result instead of aDomainError. Nothing in the suite pinned that behaviour. This adds the missing coverage.Why this was a gap
Every existing SLAYER testset constructs its parameters at
sval_r = 1.0—runtests_slayer_params.jl:13,132,runtests_slayer_riccati.jl:19,32,runtests_slayer_runner.jl:20,runtests_dispersion_coupled.jl:25,runtests_dispersion_residual.jl:25. No shipped deck has a reverse-shear rational surface either, so neither the suite nor the regression harness would notice if theabs()calls were reverted. #431's own validation was an out-of-tree scan of DIII-D shot 153072_3415, which is not reproducible in CI.What is asserted
dc_type=:larwith a nonzerodr_valis used so the check covers theWdself-consistency iteration and both critical-Δ square roots, not justtau_h.sval_rkeeps its sign in the struct — it is a diagnostic, not a magnitudesval_r = ±1.0(abs(-1.0) === 1.0, so==is the right assertion, not≈)lu > 0andtauk > 0, so theS^(1/3)that used to throw now evaluatesRegression report
The diff adds one testset and changes no file under
src/, so the harness is inert by construction and was not run. Stating this rather than pasting a run that could only report "unchanged".Validation
julia --project=. test/runtests.jl runtests_slayer_params.jl— 68/68 pass (49 before this PR)Notes for reviewers
Opened as a draft.
Suggested reviewer @matt-pharr (InnerLayer focus per
docs/development/contributors.md); assignee @d-burg. Say the word if you would rather it went to @logan-nc, who reviewed #431.Note
#298 carries a cherry-picked copy of the #431 fix ("the copy resolves away once #431 merges"). That sync is now due, and merging this first means #298 picks the test up on the same sync.