Propagate implicit solver failures#457
Merged
Merged
Conversation
868efc7 to
e9943d2
Compare
2071edf to
d692797
Compare
e9943d2 to
f652cb9
Compare
d692797 to
a29395a
Compare
This was referenced Jul 12, 2026
krystophny
added a commit
that referenced
this pull request
Jul 12, 2026
Fixes #468 ## Review context The endpoint chain is [PR 456](#456) accepted-state rollback -> [PR 457](#457) solver status -> [PR 459](#459) event location -> [PR 460](#460) exit classification and output -> [PR 461](#461) tolerance controls. [PR 462](#462) changes CI scheduling only. [PR 463](#463) adds map-resolution controls and provenance. This PR owns the first link: a rejected trial cannot replace the last accepted orbit state. Downstream solver and event work relies on that rollback contract. ## Risk tier - [ ] T0: docs, comments, small build metadata - [ ] T1: pure refactor, no behavior change intended - [ ] T2: local numerical logic - [x] T3: physics, output behavior, coordinate convention - [ ] T4: sensitive build or execution infrastructure Depends on #454. ## Correctness contract ### Intended behavior change When a symplectic microstep returns a nonzero status, keep the last accepted standard-coordinate state. Return before field-to-standard conversion, collision updates, wall intersection work, or time-index advancement. ### Behavior that must not change Accepted microsteps, wall hits, collision updates after accepted steps, the symplectic algorithm, timestep, tolerance, field evaluation, and output schema are unchanged. ### Coordinate / unit conventions Reference and integration coordinates, radians, normalized momentum, and SI output units are unchanged. ### Numerical invariants The guiding-centre Hamiltonian, source, equilibrium, symmetry, wall geometry, and boundary conditions are unchanged. Only rejected-step commit semantics change. ## Tests added - unit: `test_sympl_testfield` injects a rejected symplectic step that mutates field scratch, then verifies both macrostep paths retain `z`, `kt`, and the nonzero status - integration: existing test path - system: none - golden record: unchanged ## Golden-record impact - [x] unchanged - [ ] changed ## Failure modes considered - rejected step followed by standard-coordinate conversion; - collision update after rejection; - time-index advancement after rejection; and - the same failures in the wall-check macrostep path. The PR does not classify the rejected step as an LCFS intersection. Boundary event location and explicit output status remain separate work. ## Manual validation The regression was run with the production guards removed, then restored. It fails on the historical behavior and passes with this change. ## Verification ### Test fails on main ```text $ make CONFIG=Fast test TEST=test_sympl_testfield ERROR STOP failed symplectic step changed the accepted state 0% tests passed, 1 tests failed out of 1 ``` ### Test passes after fix ```text $ make CONFIG=Fast test TEST=test_sympl_testfield 1/1 Test #4: test_sympl_testfield ... Passed 0.06 sec 100% tests passed, 0 tests failed out of 1 ```
Base automatically changed from
fix/rejected-step-state
to
fix/ci-slow-test-gates
July 12, 2026 15:11
krystophny
added a commit
that referenced
this pull request
Jul 12, 2026
Fixes #468 ## Review context The endpoint chain is [PR 456](#456) accepted-state rollback -> [PR 457](#457) solver status -> [PR 459](#459) event location -> [PR 460](#460) exit classification and output -> [PR 461](#461) tolerance controls. [PR 462](#462) changes CI scheduling only. [PR 463](#463) adds map-resolution controls and provenance. This PR owns the first link: a rejected trial cannot replace the last accepted orbit state. Downstream solver and event work relies on that rollback contract. ## Risk tier - [ ] T0: docs, comments, small build metadata - [ ] T1: pure refactor, no behavior change intended - [ ] T2: local numerical logic - [x] T3: physics, output behavior, coordinate convention - [ ] T4: sensitive build or execution infrastructure Depends on #454. ## Correctness contract ### Intended behavior change When a symplectic microstep returns a nonzero status, keep the last accepted standard-coordinate state. Return before field-to-standard conversion, collision updates, wall intersection work, or time-index advancement. ### Behavior that must not change Accepted microsteps, wall hits, collision updates after accepted steps, the symplectic algorithm, timestep, tolerance, field evaluation, and output schema are unchanged. ### Coordinate / unit conventions Reference and integration coordinates, radians, normalized momentum, and SI output units are unchanged. ### Numerical invariants The guiding-centre Hamiltonian, source, equilibrium, symmetry, wall geometry, and boundary conditions are unchanged. Only rejected-step commit semantics change. ## Tests added - unit: `test_sympl_testfield` injects a rejected symplectic step that mutates field scratch, then verifies both macrostep paths retain `z`, `kt`, and the nonzero status - integration: existing test path - system: none - golden record: unchanged ## Golden-record impact - [x] unchanged - [ ] changed ## Failure modes considered - rejected step followed by standard-coordinate conversion; - collision update after rejection; - time-index advancement after rejection; and - the same failures in the wall-check macrostep path. The PR does not classify the rejected step as an LCFS intersection. Boundary event location and explicit output status remain separate work. ## Manual validation The regression was run with the production guards removed, then restored. It fails on the historical behavior and passes with this change. ## Verification ### Test fails on main ```text $ make CONFIG=Fast test TEST=test_sympl_testfield ERROR STOP failed symplectic step changed the accepted state 0% tests passed, 1 tests failed out of 1 ``` ### Test passes after fix ```text $ make CONFIG=Fast test TEST=test_sympl_testfield 1/1 Test #4: test_sympl_testfield ... Passed 0.06 sec 100% tests passed, 0 tests failed out of 1 ```
krystophny
added a commit
that referenced
this pull request
Jul 12, 2026
Fixes #469 ## Review context The endpoint chain is [PR 456](#456) accepted-state rollback -> [PR 457](#457) solver status -> [PR 459](#459) event location -> [PR 460](#460) exit classification and output -> [PR 461](#461) tolerance controls. [PR 462](#462) changes CI scheduling only. [PR 463](#463) adds map-resolution controls and provenance. This PR consumes PR 456's rollback rule and makes nonlinear failures observable. It does not locate or classify physical boundary events; PRs 459 and 460 own those contracts. Depends on #456. ## Risk tier - [ ] T0: docs, comments, small build metadata - [ ] T1: pure refactor, no behavior change intended - [ ] T2: local numerical logic - [x] T3: physics, output behavior, coordinate convention - [ ] T4: sensitive build or execution infrastructure ## Correctness contract Midpoint, Gauss, and Lobatto Newton solves now return distinct statuses for an exterior iterate, iteration exhaustion, and LAPACK failure. A rejected solve restores the accepted integrator and field state before returning. `dgesv` corrections are applied only when `info=0`. Successful steps retain the same nonlinear equations and update order. Their tolerances and iteration limits are unchanged. The physical problem, timestep, coordinate conventions, and units are unchanged. The top-level timestep interface and derived-type memory layout are also unchanged. Internal Newton procedure signatures gain a status result and all repository callers are rebuilt. The patch also corrects the Lobatto stage layout invariant. For stage `k>=2`, the radius is at `4*k-5`; the old negative-radius guard wrote `4*k-3`, which is the toroidal-angle slot. The correction changes only this invalid intermediate-iterate path and leaves angular periodicity unchanged. ## Tests added - zero-iteration midpoint status - exterior midpoint status - singular and nonsingular LAPACK systems - Lobatto stage-radius indexing and nonradial-component preservation ## Verification ### Test fails before fix ```text test_newton_solver_status.f90: Error: Symbol 'symplectic_step_maxiter' has no IMPLICIT type test_newton_solver_status.f90: Error: More actual than formal arguments in procedure call ninja: build stopped: subcommand failed ``` The pre-fix Lobatto guard also changes the stage-2 toroidal-angle component instead of flooring its negative radius. ### Test passes after fix ```text test_newton_solver_status ... Passed 100% tests passed, 0 tests failed out of 1 make test-nopy 100% tests passed, 0 tests failed out of 54 ``` The full gate used four build CPUs and completed with 10.5 GB peak memory.
krystophny
added a commit
that referenced
this pull request
Jul 13, 2026
Fixes #471 ## Review context The endpoint chain is [PR 456](#456) accepted-state rollback -> [PR 457](#457) solver status -> [PR 459](#459) event location -> [PR 460](#460) exit classification and output -> [PR 461](#461) tolerance controls. [PR 462](#462) changes CI scheduling only. [PR 463](#463) adds map-resolution controls and provenance. This PR owns the downstream terminal-event contract: `raw microstep -> collocated event -> physical/numerical classification -> restart record -> text and NetCDF output` Every stage carries the same event time, phase-space state, boundary source, and exit class. This is why the change crosses integration, restart, and output code. Separates physical LCFS and wall losses from numerical exits throughout tracing, restart, text output, and NetCDF output. Map-boundary metadata controls whether `s=1` is the physical LCFS. Wall intersection is checked on every microstep and commits one interpolated phase-space event, including fractional time, momentum, pitch, exact hit position, and incidence. Restart reuse requires matching exit-code/time evidence; ambiguous legacy early exits and wall exits are retraced. Numerical exits are neither lost nor confined and are reported through a separate unresolved fraction. The equilibrium, particle ensemble, energy, collision model, trace duration, field periods, Hamiltonian, source distribution, coordinate handedness, units, physical wall, LCFS, and boundary conditions are unchanged. The symplectic integrator type adds three real event diagnostics, so all consumers are rebuilt together. ## Verification Failing before: ```text Numerical exits could be counted as confined. Extended map boundaries could be labeled LCFS. Wall hits could span multiple microsteps and mix endpoint states. Legacy restart inference could convert numerical failures to physical losses. ``` Passing after: ```text Static: OK Build: OK Tests: OK Lint: OK All stages passed ``` Behavioral tests cover map metadata, exit schemas, NetCDF diagnostics, interruption-safe restart ordering, legacy retracing, exact wall-segment intersection, periodic wall coordinates, and full-state interpolation. ## Golden-record review Failing before the reference update: ```text golden_record_boozer_ncsx: 104 mismatched entries golden_record_meiss_coils: 156 mismatched entries, including numerical-failure NaNs ``` Passing after: ```text Static: OK (171 modules, 171 changed, 171 affected) Build: OK Tests: OK Lint: OK All stages passed ``` The reference build applies the reviewed physical-exit patch before comparison. The comparator remains strict over every column, treats paired NaNs as equal, and keeps the existing numerical tolerances. Sanity tests cover matching and mismatching NaNs.
krystophny
added a commit
that referenced
this pull request
Jul 13, 2026
Fixes #472 ## Review context The endpoint chain is [PR 456](#456) accepted-state rollback -> [PR 457](#457) solver status -> [PR 459](#459) event location -> [PR 460](#460) exit classification and output -> [PR 461](#461) tolerance controls. [PR 462](#462) changes CI scheduling only. [PR 463](#463) adds map-resolution controls and provenance. This PR changes configuration only. It exposes the two acceptance thresholds already used by PR 459 without changing solver tolerances, event arithmetic, or defaults. Adds independent namelist controls for the dimensionless fractional-step bracket and radial residual tolerances used to accept an `s=1` event. The default `-1` preserves the existing `relerr`-derived behavior exactly; positive values allow one-parameter event-location refinement without changing the nonlinear solver tolerance. Configuration validation uses the binary64 exponent bits because the production module is compiled with `-ffast-math`, under which `ieee_is_finite` can be optimized to true. NaN and infinity overrides are rejected under the actual production flags. The physical model, solver arithmetic, public interfaces, and memory layout are unchanged. Only the event acceptance threshold changes when a positive override is requested. ## Verification Failing before: ```text Event-location tolerance could not be varied independently of relerr. NaN overrides bypassed the nonpositive-value check under -ffast-math. Nested test callbacks failed -Werror=trampolines. ``` Passing after on the exact dependent stack: ```text loose configured tolerance: boundary accepted tight configured radial tolerance: unresolved status with exact rollback valid event configuration: accepted fractional NaN: rejected fractional positive infinity: rejected radial NaN: rejected Static: OK (155 modules) Build: OK Tests: OK Lint: OK All stages passed ``` ## Golden-record status The dependent stack now applies the reviewed physical-exit reference patch. Golden comparisons pass with unchanged tolerances; this PR does not alter default-input results.
krystophny
added a commit
that referenced
this pull request
Jul 13, 2026
Fixes #474 ## Review context The endpoint chain is [PR 456](#456) accepted-state rollback -> [PR 457](#457) solver status -> [PR 459](#459) event location -> [PR 460](#460) exit classification and output -> [PR 461](#461) tolerance controls. [PR 462](#462) changes CI scheduling only. This PR is a separate reproducibility contract for canonical maps. A resolution scan requires caller-controlled dimensions and tolerance. Reinitialization must discard the prior map, and results must record the selected controls. Without those properties, a nominal convergence scan can reuse stale state or produce output that does not identify its discretization. ## Summary - expose independent grid dimensions and transformation-ODE controls for Meiss and Albert maps - reset configuration and spline storage on every initialization and field-mode switch - reject invalid map dimensions and tolerances before allocation - record map, event, and Fourier controls in NetCDF results - verify namelist propagation, repeated Meiss/Albert construction, ALBERT-to-MEISS cleanup, coordinate roundtrip, radial orientation, field-period seams, and finite field derivatives The VMEC equilibrium, physical field, guiding-center Hamiltonian, source distribution, boundary, coordinate handedness, angular periods, and units are unchanged. Existing Fortran calls remain source-compatible through trailing optional arguments; consumers must rebuild their module files. Canonical tolerance validation reuses the binary64 finite-value check required by the production `-ffast-math` flags. ## Verification Failing before: ```text canonical grid and transformation tolerance were fixed in source custom-then-default initialization retained the prior configuration ALBERT-to-MEISS switching retained Albert spline storage results.nc did not identify the canonical discretization ``` Passing after on the exact PR stack with deterministic floating point: ```text HDF5/NetCDF smoke checks: PASSED Static: OK (171 modules, 171 changed, 171 affected) Build: OK Tests: OK Lint: OK All stages passed ``` The validated run completed in 7:46 with 32.6 GiB peak RSS. The restack changes configuration rejection and shared regression tests; finite valid inputs and all production solver paths are unchanged. ## Golden-record status Failing before the canonical reference update, the deterministic `boozer_ncsx` comparison retained two final-coordinate mismatches after the physical-exit reference was applied. The generated canonical-resolution reference patch reproduces the configured map lifecycle on the reference build. A fresh deterministic comparison then matched exactly at the existing tolerances, and the full affected-test run passed.
krystophny
added a commit
that referenced
this pull request
Jul 13, 2026
Fixes #470 ## Review context The endpoint chain is [PR 456](#456) accepted-state rollback -> [PR 457](#457) solver status -> [PR 459](#459) event location -> [PR 460](#460) exit classification and output -> [PR 461](#461) tolerance controls. [PR 462](#462) changes CI scheduling only. [PR 463](#463) adds map-resolution controls and provenance. This PR converts explicit solver/domain statuses into one transactional, converged map-boundary event. It does not decide whether that boundary is a physical LCFS or numerical map limit; PR 460 owns that classification. Implements one transactional boundary wrapper for the symplectic modes while preserving the public procedure-pointer ABI. Rejected nonlinear trials restore the accepted integrator and field. A physical boundary is accepted only when both the one-sided time bracket and radial residual converge. The guiding-centre Hamiltonian, source distribution, field, outer boundary `s=1`, stellarator symmetry, angular conventions, and units are unchanged. Euler and midpoint axis crossings preserve `(r,theta) ~ (-r,theta+pi)` and refresh field state; Gauss rejects unresolved negative collocation stages instead of committing an inconsistent chart. GPU benchmarking rejects CPU traces that reach behavior not implemented on device. The head also contains the reviewed dependent changes from merged stack PRs #460, #461, and #463. They are carried here because those PRs targeted this feature branch; this PR is the sole merge into `main`. ## Verification Failing before: ```text Euler solver exhaustion could return success. A solver-basin transition could be accepted as a physical boundary. Rejected Gauss post-solve exits could retain mutated state. ``` Passing after: ```text Static: OK (171 modules, 171 changed, 171 affected) Build: OK Tests: OK Lint: OK All stages passed ``` SPECTRE compatibility: `test_spectre_sympl_volume`, `test_spectre_sympl_crossing`, and `test_spectre_sympl_crossing_invariants` pass. Five 16-thread construction runs were bit-identical with maximum B-identity residual `7.3562e-7`. The SPECTRE midpoint map remains inline and owns its interfaces; no SPECTRE source was reformatted. Focused behavioral tests cover explicit solver statuses, exact rollback, a manufactured `s(t)=0.9+t` crossing, mixed solver basins, axis-chart behavior, and all supported production boundary modes.
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.
Fixes #469
Review context
The endpoint chain is PR 456 accepted-state rollback -> PR 457 solver status -> PR 459 event location -> PR 460 exit classification and output -> PR 461 tolerance controls. PR 462 changes CI scheduling only. PR 463 adds map-resolution controls and provenance.
This PR consumes PR 456's rollback rule and makes nonlinear failures observable. It does not locate or classify physical boundary events; PRs 459 and 460 own those contracts.
Depends on #456.
Risk tier
Correctness contract
Midpoint, Gauss, and Lobatto Newton solves now return distinct statuses for an exterior iterate, iteration exhaustion, and LAPACK failure. A rejected solve restores the accepted integrator and field state before returning.
dgesvcorrections are applied only wheninfo=0.Successful steps retain the same nonlinear equations and update order. Their tolerances and iteration limits are unchanged. The physical problem, timestep, coordinate conventions, and units are unchanged. The top-level timestep interface and derived-type memory layout are also unchanged. Internal Newton procedure signatures gain a status result and all repository callers are rebuilt.
The patch also corrects the Lobatto stage layout invariant. For stage
k>=2, the radius is at4*k-5; the old negative-radius guard wrote4*k-3, which is the toroidal-angle slot. The correction changes only this invalid intermediate-iterate path and leaves angular periodicity unchanged.Tests added
Verification
Test fails before fix
The pre-fix Lobatto guard also changes the stage-2 toroidal-angle component instead of flooring its negative radius.
Test passes after fix
The full gate used four build CPUs and completed with 10.5 GB peak memory.