Repository navigation
Tearing - BUGFIX! - Reject GGJ tearing models at parse time and stop swallowing stage errors - #547
Merged
Merged
Conversation
matt-pharr
force-pushed
the
bugfix/reject-ggj-tearing
branch
from
October 20, 2026 12:22
7febb5d to
b6bbb9a
Compare
…swallowing stage errors A deck with inner_model = "ggj_*" passed validation, then solve(TearingProblem, GGJModel) errored and run_slayer_stage's try/catch logged it, so the run finished with no Tearing/ group. validate now rejects the ggj_* models, since GGJ tearing growth rates are not implemented in v2.0.0. run_slayer_stage no longer catches errors, so a failing tearing stage stops the run after every earlier stage has written its output. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
logan-nc
force-pushed
the
bugfix/reject-ggj-tearing
branch
from
October 22, 2026 07:19
b6bbb9a to
29a7577
Compare
logan-nc
requested changes
Oct 22, 2026
logan-nc
left a comment
Collaborator
There was a problem hiding this comment.
OK. My one nit here is that putting an exact version number into the source code is a recipe for getting stale messaging. Just remove the specific version and say it is not currently supported
…ion messages Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdRsGGg4YRG6E9fgLGbpdF
logan-nc
approved these changes
Oct 22, 2026
Collaborator
|
@matt-pharr two post-merge notes:
|
Collaborator
Author
|
Thanks @d-burg, not intended. The rejection should only apply when tearing is enabled. I'll open a follow-up that skips the inner-model check when |
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
[SLAYER] inner_model = "ggj_ray","ggj_shooting"or"ggj_galerkin"now fails at the start of the force-free-states stage (before any solve, and not at all on an equilibrium-only run), with a message that GGJ tearing growth rates are not currently supported; useinner_model = "slayer_fitzpatrick". This is revisable whenever @d-burg merges in his work fixing the GGJ normalizations for tearing applications. A tearing stage that fails for any other reason now stops the run instead of logging an error and finishing.A deck that asked for a GGJ tearing model used to finish "successfully" with no
Tearing/group, because the stage caught and logged the "not implemented" error. GGJ tearing models are now rejected up front, and tearing-stage errors reach the user; every earlier stage has already written its output when the tearing stage runs.Regression report
At the current head, after #536 merged and the base became develop:
--refs 9365f0d6c,9fadd79c5 --cases diiid_slayer_n1,diiid_n1(feynman, fresh cache). Every quantity is unchanged.Earlier run against the old stack base:
Compared against the stack base,
--refs c8b68e4ba,b6bbb9ae9(refactor/ggj-ray-galerkin-bvp,bugfix/reject-ggj-tearing), ondiiid_slayer_n1anddiiid_n1_riccati, the two cases that run SLAYER.Notes for reviewers
refactor/ggj-ray-galerkin-bvp, and the diff shows only this change; nothing here depends on InnerLayer.GGJ - REFACTOR! - Solve the :ray BVP with the Galerkin assembly and use one parity convention #536's code. GitHub retargets this PR todevelopwhen InnerLayer.GGJ - REFACTOR! - Solve the :ray BVP with the Galerkin assembly and use one parity convention #536 merges.run_slayer_stageis removed, not narrowed. SLAYER runs last in both theforce_terminationpath and the full pipeline, so the force-free-states, PE, KineticForces and ErrorFields outputs are already on disk when it runs. Letting errors through loses only theInfo/Runtimesrecord and the in-REPL return value. Narrowing would mean listing "expected" failures, and there is no such list. The deck now behaves like the API, which already threw.validate, so bothslayer_control_from_toml(the deck) andsolve(::TearingProblem, ...)rejectggj_*.run_slayer_from_inputsvalidates a control with the default model, so its low-level GGJ path is unchanged. Its cleanup is a separate PR._build_inner_modelare deleted because nothing can reach them now.solve(::TearingProblem, ::GGJModel)keeps its guard, with the message changed to match.🤖 Generated with Claude Code