Unification of the three integrators - PR #1 - #381
Merged
Conversation
Replace the use_riccati / use_parallel / parallel_threads / populate_dense_xi flag set with a single integrator = "forward" | "riccati" | "galerkin" key plus an nchunks knob, per the ForceFreeStates modularization plan (PR 1 of 5). - Delete the standalone serial-Riccati driver and the populate_dense_xi dense-xi re-run; no integration result is merged with another. - Rename serial_eulerlagrange_integration -> forward_eulerlagrange_integration and give the STRIDE propagator driver the freed riccati_ name. - Chunk sizing no longer reads Threads.nthreads(), so Riccati outputs are identical at any thread count; nchunks is the only tunable and clamps up to the singular-surface floor with a warning. - Kinetic runs now error on the Riccati path rather than silently mis-crossing. - The four retired keys join _DEPRECATED_FFS_KEYS and the toml-no-deprecated-keys hook, so old decks and gpec.h5 replays warn and continue. - New examples/DIIID-like_riccati_deltaprime_example plus the diiid_n1_riccati regression case keep the delta-prime matrix covered once the DIIID-like deck moves to the forward integrator. Verified on origin/develop: full suite 59 testsets / 2044 assertions / 0 failures; docs build clean; harness diiid_n1, solovev_n1 and diiid_slayer_n1 all unchanged; delta-prime bit-identical between -t 1 and -t 4. REFACTOR_PLAN.md is amended in the same commit: #363 and #345 merged before this sequence branched, so the plan now records that it builds on the new HDF5 schema. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
matt-pharr
force-pushed
the
refactor/riccati-unification
branch
from
August 15, 2026 02:14
7fc592e to
ab58849
Compare
matt-pharr
marked this pull request as ready for review
August 15, 2026 02:45
3 tasks
logan-nc
approved these changes
Aug 15, 2026
logan-nc
left a comment
logan-nc
left a comment
Collaborator
There was a problem hiding this comment.
great clarity win!
Only a few minor docs comments
| `eulerlagrange_integration` is the baseline driver. It integrates the EL ODE directly in | ||
| ``(U_1, U_2)`` using Tsit5 with adaptive step control. Near each rational surface the | ||
| `forward_eulerlagrange_integration` is the baseline driver. It integrates the EL ODE directly | ||
| in ``(U_1, U_2)`` using Tsit5 with adaptive step control. Near each rational surface the |
Collaborator
There was a problem hiding this comment.
Is this Tsit5 claim out of date?
| ``\Delta'`` matrix. | ||
|
|
||
| ### Standard integration | ||
| ### Forward integration |
Collaborator
There was a problem hiding this comment.
We should cite the main 2016 Glasser DCON reference in this section of the docs and make it clear this is our version of that approach. Similarly, we should explicitly say Riccati is our implementation of the STRIDE approach and cite it's papers.
| than integrating the quadratic Riccati ODE directly (which blows up when ``|S|`` is large), | ||
| the code integrates the linear EL system with `sing_der!` as the RHS and recovers | ||
| ``S = U_1 U_2^{-1}`` via periodic renormalization — an approach that is mathematically | ||
| equivalent to O(Δψ) but uses the ODE solver's full 5th-order accuracy. Renormalization is |
Collaborator
There was a problem hiding this comment.
Again, the "5th-order accuracy" might be out of date here. @d-burg changed the integrators a while back
…ty-module FFS - REFACTOR - Extract local stability into standalone LocalStability module (refactor plan PR 2)
matt-pharr
enabled auto-merge
August 15, 2026 19:12
matt-pharr
disabled auto-merge
August 15, 2026 19:13
matt-pharr
enabled auto-merge
August 15, 2026 19:20
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
2 participants
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.
PR 1 of the refactor sequence in
REFACTOR_PLAN.md(§3). Delivery: this PR targetsdevelop; #387 (LocalStability) stacks on it, and the remaining work lands as a single combined interface PR onrefactor/forcefreestates-result(three slice-pure commits: result struct + consumers + standalone Galerkin, staged main, solve API — the original PRs 3–5, collapsed).What this does
populate_dense_xi/_populate_dense_xi_via_serial_el!, andparallel_threads— no merging of two integration results anywhere (plan D3).integrator = "forward" | "riccati"in[ForceFreeStates]("galerkin"reserved, errors until PR 4). Old keys (use_riccati,use_parallel,parallel_threads,populate_dense_xi) are warn-and-ignored via_DEPRECATED_FFS_KEYS, so pre-refactor gpec.h5 replays keep working; new TOMLs are blocked from using them by the pre-commit hook.julia -tprovides; the only tunable isnchunks(0 = auto, derived frommsingalone — chunk sizing never consultsThreads.nthreads(), so Riccati outputs are thread-count independent; new tests pin this).serial_eulerlagrange_integration→forward_eulerlagrange_integration; the STRIDE driver now owns thericcati_eulerlagrange_integrationname. Erroneous "shooting" terminology renamed to "forward" (legitimate STRIDE/GGJ shooting terminology untouched).kinetic_factor > 0now errors with a clear message (plan D4: kinetic is Forward-only).forwardfor PE/kinetic decks,riccatifor Δ′/stability decks; gal decks stayriccati(their PE consumes the gal-matched solution, not the integrator's).examples/DIIID-like_riccati_deltaprime_example/(references the DIIID geqdsk by relative path, no duplication) +regression-harness/cases/diiid_n1_riccati.toml: the canonicalSingularSurfaces/delta_prime_matrixtrack moves there;diiid_n1keeps the ξ / perturbed-equilibrium quantities on the forward integrator.stability.md, control-struct docstrings), benchmark scripts, andREFACTOR_PLAN.mdamendments (upstream ALL - REFACTOR - Module-mirroring CamelCase HDF5 schema (issue #226, PR A) #363/VACUUM - Migrate surface current calculation into the VACUUM module #345 absorbed; progress checklist updated) included.Verification
runtests_eulerlagrange.jl,runtests_riccati.jl,runtests_parallel_integration.jl(incl. new thread-independence /nchunkssteering / clamp-warning tests),runtests_rerun_from_h5.jl(incl. new deprecated-key replay-warn test).diiid_n1,diiid_n1_riccatifirst baseline,solovev_n1) — pasted here before this PR leaves draftbuild_docs_local.jl)This PR must not be merged without a third-party human reviewer's approval — non-negotiable (see
REFACTOR_PLAN.md§8).🤖 Generated with Claude Code