Restore liberal Newton continuation outside SPECTRE#495
Closed
krystophny wants to merge 8 commits into
Closed
Conversation
5 tasks
## 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: parallelism, GPU, dependency, CI, or security-sensitive build logic ## Correctness contract ### Intended behavior change Add a side-effect-free Fortran API and a Python API that compute canonical bounce/transit and mean toroidal angular frequencies for trapped and passing guiding-centre orbits. Support one or multiple completed poloidal cycles, return cycle spreads and explicit status metadata, and expose interpolated tip/toroidal cut tracing through Python. ### Behavior that must not change Existing orbit tracing, classification, NetCDF output, and golden-record results are unchanged. The core frequency routine copies the caller's tracer and does no file I/O. ### Coordinate / unit conventions The source-level Fortran routine accepts SIMPLE integration coordinates. The flat/Python boundary accepts `[s, theta, phi, v/v0, lambda]` in public reference coordinates and performs `ref_to_integ`. Angles and displacements are radians, periods are seconds, and frequencies are angular frequencies in rad/s. `omega_b` is positive; `omega_phi` is signed. ### Numerical invariants Trapped periods use successive negative-to-positive parallel-velocity crossings. Passing periods use successive signed `2*pi` advances of unwrapped poloidal angle. Event time and toroidal displacement are interpolated within the crossing step. Multi-cycle results report means and sample standard deviations. ## Tests added - unit: Fortran invalid-option status and Python argument validation - integration: native trapped/passing, three-cycle frequency computation - system: Python trapped multi-cycle frequency and interpolated tip-cut APIs - golden record: unchanged ## Golden-record impact - [x] unchanged - [ ] changed ## Failure modes considered Invalid options, orbit loss, integrator errors, and maximum-step exhaustion return distinct statuses. Unknown orbit class remains explicit. The cut wrapper rejects unsupported cut types and non-symplectic integration. ## Manual validation Validated trapped and passing VMEC seeds through the native API; validated the Python frequency and cut interfaces against the compiled f90wrap backend. ## Verification - `make test TEST=^test_orbit_frequencies$ CONFIG=Fast VERBOSE=0` - `make test TEST=^test_simple_api$ CONFIG=Fast VERBOSE=0` (24 API cases pass) - Full non-regression suite: 75/76 targets pass after installing the optional plotting dependency. The remaining `test_orbit_macro` failure is pre-existing: it accesses `pysimple._backend`, which current `main` does not export. - `git diff --check`
## Summary - add the explicit target-order dependency required when `diag_albert.f90` consumes `pyplot_module.mod` from the separate `pyplot` target - prevent clean parallel Ninja builds from compiling `diag_albert` before the module file exists ## Evidence The CGAL wall CI job on PR #499 failed in a clean parallel build with: ``` Fatal Error: Cannot open module file ‘pyplot_module.mod’ ``` The existing link edge orders final linking but did not reliably order compilation across targets. This PR adds no source or runtime behavior change. ## Validation - clean `fo build` passed - `fo` passed all stages in 308.3 s
Member
Author
|
Closing after critical review: the generic |
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.
Restores the historical default warning behavior for generic SIMPLE paths while keeping the policy explicit per integrator.
Stacked on the recovery/accounting PR so concerns remain reviewable.