Repository navigation
Tearing - BUGFIX - Check the inner model and profile file only when tearing is enabled, and fail before the solve - #567
Conversation
…earing is enabled, and fail before the solve The layer-overlap scan parses every [SLAYER] section before the force-free-states stage, so the GGJ rejection aborted decks that keep a disabled [SLAYER] section only for its profiles. validate now rejects a GGJ inner model only when tearing is enabled; an unknown inner_model is still an error either way. With tearing enabled, an empty or unreadable profile_file now fails at the start of the force-free-states stage instead of only warning there and failing after every later stage. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
d-burg
left a comment
There was a problem hiding this comment.
Thanks! I couldn't break either fix: a disabled [SLAYER] section can keep a GGJ inner_model, typos are still rejected, and an enabled section with an empty or missing profile_file now fails before the force-free-states solve instead of at the end. A few small claude things for your consideration, none blocking:
- Where the early check runs. It depends on
_overlap_slayer_inputsstaying an eager argument toprepare_force_free_states!. Withmainbeing restructured, it would be easy to make that call lazy (or gate it onpsilim_from_layer_overlap) and quietly lose the check. Suggestion 1 adds a comment; a pipeline-level test, or the post-parse check you mentioned, would be sturdier. - One behavior change for the release note.
TearingProblemreturns an empty result before loading profiles when there are no surfaces, so a deck with tearing enabled, a badprofile_fileand no singular surfaces in range used to finish and now aborts. Content errors in an existing file (missingn_e/T_e/T_i,zeff < 1) also fire early now. I think that's right, but it's worth a clause. - Wording.
run_local_stabilityruns before the check, so "before the force-free-states solve" is more exact than "at the start of the force-free-states stage", and matches the title.
Suggestions 2 and 3 fix the inner_model docstring's "only option" and pin the disabled, empty-file branch in a test.
| ffs_start = time() | ||
|
|
||
| locstab, ballooning_boundary = run_local_stability(ctrl, equil) | ||
| metric, mats, layer_overlap = prepare_force_free_states!(intr, ctrl, equil, kf_ctrl, kinetic_profiles; |
There was a problem hiding this comment.
The early failure only works because _overlap_slayer_inputs is evaluated eagerly ahead of the solve. A note here protects it when main is restructured.
| # _overlap_slayer_inputs also rejects an enabled [SLAYER] section's unusable profile_file, so keep it ahead of the solve. | |
| metric, mats, layer_overlap = prepare_force_free_states!(intr, ctrl, equil, kf_ctrl, kinetic_profiles; |
| - `inner_model` -- `:slayer_fitzpatrick` (default and only option; when `enabled`, | ||
| the `:ggj_*` models are rejected because GGJ tearing growth rates are not implemented) |
There was a problem hiding this comment.
"Only option" now contradicts the disabled section exception.
| - `inner_model` -- `:slayer_fitzpatrick` (default and only option; when `enabled`, | |
| the `:ggj_*` models are rejected because GGJ tearing growth rates are not implemented) | |
| - `inner_model` -- `:slayer_fitzpatrick` (default and only supported option; the `:ggj_*` names | |
| are accepted only when `enabled = false`, because GGJ tearing growth rates are not implemented) |
| # A disabled section feeds the scan whatever its inner model, and an unreadable file only skips the scan; | ||
| # with tearing enabled the same file is an error, raised here rather than after every other stage. | ||
| @test overlap_inputs(Dict("inner_model" => "ggj_ray")) !== nothing | ||
| @test (@test_logs (:warn, r"Layer-overlap scan skipped") match_mode = :any overlap_inputs(Dict("profile_file" => "missing.h5"))) === nothing | ||
| @test_throws ErrorException overlap_inputs(Dict("enabled" => true, "profile_file" => "missing.h5")) | ||
| @test_throws ErrorException overlap_inputs(Dict("enabled" => true, "profile_file" => "")) |
There was a problem hiding this comment.
Claude: this pins the disabled side of the new empty-file short-circuit, and the comment now matches what's exercised (a missing file, not an unreadable one).
| # A disabled section feeds the scan whatever its inner model, and an unreadable file only skips the scan; | |
| # with tearing enabled the same file is an error, raised here rather than after every other stage. | |
| @test overlap_inputs(Dict("inner_model" => "ggj_ray")) !== nothing | |
| @test (@test_logs (:warn, r"Layer-overlap scan skipped") match_mode = :any overlap_inputs(Dict("profile_file" => "missing.h5"))) === nothing | |
| @test_throws ErrorException overlap_inputs(Dict("enabled" => true, "profile_file" => "missing.h5")) | |
| @test_throws ErrorException overlap_inputs(Dict("enabled" => true, "profile_file" => "")) | |
| # A disabled section feeds the scan whatever its inner model, and a missing or empty file only skips the scan; | |
| # with tearing enabled the same file is an error, raised here rather than after every other stage. | |
| @test overlap_inputs(Dict("inner_model" => "ggj_ray")) !== nothing | |
| @test (@test_logs (:warn, r"Layer-overlap scan skipped") match_mode = :any overlap_inputs(Dict("profile_file" => "missing.h5"))) === nothing | |
| @test (@test_logs overlap_inputs(Dict("profile_file" => ""))) === nothing | |
| @test_throws ErrorException overlap_inputs(Dict("enabled" => true, "profile_file" => "missing.h5")) | |
| @test_throws ErrorException overlap_inputs(Dict("enabled" => true, "profile_file" => "")) |
|
Thanks, Daniel. All three suggestions are applied in dc8356a.
|
…bled-checks # Conflicts: # src/GeneralizedPerturbedEquilibrium.jl
Release note
A
[SLAYER]section withenabled = falsemay again name a GGJinner_model, for example when the section is kept only for the layer-overlap scan's profiles; GGJ tearing models are rejected only when tearing is enabled. With tearing enabled, an empty or unreadableprofile_filenow stops the run before the force-free-states solve, instead of warning there and failing in the tearing stage after every other stage has run. The same holds when no singular surface is in range, where the run used to finish with an empty tearing result, and for content errors in an existing file (a missingn_e,T_eorT_i, orzeff < 1).Regression report
Run on ITER SDCC (sirius, 8 CPUs per task) with
regress_fanout.sh --refs develop,bugfix/tearing-enabled-checks --cases diiid_slayer_n1,diiid_slayer_n1_coupled,diiid_n1_riccati, after merging develop.Notes for reviewers
Follow-up to @d-burg's post-merge notes on #547.
validatekeeps rejecting an unknowninner_modelwhether or not tearing is enabled, so typos are still caught; the three GGJ names count as known. Only the "GGJ tearing is not supported" rejection waits forenabled = true._overlap_slayer_inputsis where the deck first parses[SLAYER], before the force-free-states stage. With tearing enabled it now lets the profile-loading error through (an empty path or an unreadable file); with tearing disabled it still skips the scan with a warning. Checking earlier, right after the TOML is read, would mean restructuringmain, which Repo - API! - Time stages in main and drop the runtimes keyword #552 and §7D are changing.🤖 Generated with Claude Code