Skip to content

Preserve accepted state after rejected orbit steps#456

Merged
krystophny merged 2 commits into
fix/ci-slow-test-gatesfrom
fix/rejected-step-state
Jul 12, 2026
Merged

Preserve accepted state after rejected orbit steps#456
krystophny merged 2 commits into
fix/ci-slow-test-gatesfrom
fix/rejected-step-state

Conversation

@krystophny

@krystophny krystophny commented Jul 11, 2026

Copy link
Copy Markdown
Member

Fixes #468

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 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
  • 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

  • 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

$ 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

$ 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 krystophny added tier/T3 physics or output behavior size/S review size up to 100 changed lines labels Jul 11, 2026
@krystophny krystophny force-pushed the fix/rejected-step-state branch from 70a27a3 to 868efc7 Compare July 11, 2026 20:46
@krystophny krystophny force-pushed the fix/rejected-step-state branch from 868efc7 to e9943d2 Compare July 11, 2026 22:29
@krystophny krystophny changed the base branch from main to fix/ci-slow-test-gates July 11, 2026 22:29
@krystophny krystophny force-pushed the fix/ci-slow-test-gates branch from 928a37c to d184450 Compare July 12, 2026 00:19
@krystophny krystophny force-pushed the fix/rejected-step-state branch from e9943d2 to f652cb9 Compare July 12, 2026 00:20
@krystophny krystophny merged commit 638528d into fix/ci-slow-test-gates Jul 12, 2026
7 checks passed
@krystophny krystophny deleted the fix/rejected-step-state branch July 12, 2026 15:11
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 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S review size up to 100 changed lines tier/T3 physics or output behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant