FFS - REFACTOR - Publish solves as ForceFreeStatesResult; staged main; solve API (refactor plan interface PR) - #393
Conversation
…Galerkin integrator
Interface PR, commit (a) of 3 (REFACTOR_PLAN.md §5). One immutable result struct
is now the only thing that crosses the ForceFreeStates module boundary:
- ForceFreeStatesResult carries the mode space, domain, assembly products, and
per-formalism products as Union{Nothing,T} capability fields; consumers
(PerturbedEquilibrium, HDF5 writer, SLAYER, write_imas, KineticForces) gate
with require/require_solution and warn-and-skip instead of erroring.
- SolutionProfiles is the single ξ interchange type (Ξ, Ξ′, Ξ_s always populated):
forward publishes its dense axis-basis solution; matched Galerkin publishes its
native-grid solution directly; Riccati publishes none until resonant
reconstruction lands. Closure is explicit: result.closure (:ideal|:matched) and
result.bpen (zeros under ideal closure) are unconditional; result.wp carries the
fixed-boundary plasma energy matrix even when vac_flag=false.
- integrator="galerkin" becomes a standalone formalism (own vacuum wv via the
compute_scaled_wv extraction from free_run); additive gal_flag is removed and
deprecated; the four gal decks retoml to the new integrator.
- Deleted: gal_matched_odestate (OdeState shim) and the driver's gal/odet
arbitration. _chord_solution_at retained as an uncalled helper pending the
Riccati eigenfunction-reconstruction decision.
- main returns (; ffs, pe, slayer). HDF5 dataset paths unchanged; forward and
riccati outputs byte-identical to the stack base, Galerkin-group datasets
identical between the old additive and new standalone paths.
Co-Authored-By: Claude Opus <noreply@anthropic.com>
… one DeltaPrimeData
…eld source algebra, perturbed_equilibrium
…gal raw basis to debug output
…nt for the PE split Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e in the capability matrix Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@logan-nc This PR is now done and ready for review. It is a hefty PR but the reorganization is worth it IMO. This results in the integrators being abstracted behind a new x = EulerLagrangeProblem(), solve(x, integration method) problem interface that is julia-y (what is the julia equivalent of pythonic?). It is most of the way there to getting the actual main function to be trivially small. The next PR will actually get us all the way there to the main() function being tiny and super readable and to the point where the toml file will be 1:1 with args in the different module functions. Once we get there, I can make a jupyter notebook example of how to use the new syntax in a way that is more usable than main() from toml. PS I do solemnly swear that my eyes touched every line of code change :) |
|
Merged develop (now including #367 + #395). Resolution was mechanical: deprecated-key/hook unions and taking #367's removal of the 🤖 Generated with Claude Code |
|
One consequence of the unified Galerkin's Δ′ moves from Concretely: flip a deck's In #397 I pin the deck's declared Cheapest fix I can see: use the backend slot the schema already reserves — Happy to be told this is handled somewhere I've missed. Separately: this merges cleanly into both my branches and isn't a silent semantic break — on |
jhalpern30
left a comment
jhalpern30
left a comment
There was a problem hiding this comment.
Gave this a super quick skim since I see this more along the lines of a hackathon where the goal is just to get this working/intergrated asap to avoid merge conflicts and it can be cleaned up later
Just made a few comments, overall the changes look great
| - `wall_settings::Vacuum.WallShapeSettings` - Wall shape settings for vacuum calculations | ||
| """ | ||
| @kwdef mutable struct ForceFreeStatesInternal | ||
| @kwdef mutable struct ForceFreeStatesInternal <: ModeSpace |
There was a problem hiding this comment.
Just wanted to flag that this is a bandaid on something I didn't get to during the hackathon - the FFSInternal struct is one of the worst remaining in terms of entries getting changed throughout the code and its almost treated like a global. One of Claude's suggestions was to break it up into a ModeGeometry struct (so ModeSpace here) for the first third-ish of the data, SingularSurfs struct for the middle section, and IntegrationLimits for the last bits. No need to do that in this PR, but wanted to mention
There was a problem hiding this comment.
Hmm I do really like this proposal. The ModeSpace thing was the easiest way I found to allow multiple structs that have these mode bounds in them to be accepted by functions that need them. This is probably better. I will do this once you add your commits from your FourFitVars restructure here. Please let me know when that is done on slack @jhalpern30
| point clouds the free-boundary result carries to HDF5. `wv` aliases `vac.wv`, which is | ||
| scaled in place. | ||
| """ | ||
| function compute_scaled_wv(ctrl::ForceFreeStatesControl, equil::Equilibrium.PlasmaEquilibrium, intr::ForceFreeStatesInternal) |
There was a problem hiding this comment.
Was initially going to mention that we should just move the singfac scaling into Vacuum. But upon second look, the wvspline in the edge has a seemingly legitimate reason to want the unscaled matrix so I will recant my request but comment as an FYI
| 4. Forward DFT back to m-space | ||
| """ | ||
| function set_perturbation_data!(kf_intr::KineticForcesInternal, pe_state, ffs_intr, | ||
| function set_perturbation_data!(kf_intr::KineticForcesInternal, pe_state, ffs, |
There was a problem hiding this comment.
I believe these are going to create type-instabilities - can this be avoided?
| response_vector::Vector{ComplexF64}, | ||
| flux_matrix::Matrix{ComplexF64}, | ||
| ForceFreeStates_results::OdeState, | ||
| ForceFreeStates_results::SolutionProfiles, |
There was a problem hiding this comment.
So we have ffs which is a ForceFreeStatesResult instance, and we have ForceFreeStates_results which is a SolutionProfiles instance? lol...
There was a problem hiding this comment.
Yes this could stand to be cleaned up before merging. Will do that.
|
@d-burg noted on the h5 file reflecting the Delta' provenance. Do you think we should move Delta' into the per-integrator folder? One of the long term end goals here is to eliminate the need for the h5 file at all and have the h5 only be produced if requested or if running via toml. That way nothing is automatically saved to disk and users can script with GPEC as they please, and only save what they need. There should be no need to run multiple integrators in a single GPEC run at the end, so at that point we could collapse everything in the hdf5 and just have a single field that says what integrator was used. |
…s, rename ForceFreeStates_results to solution Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3 participants
Third and final PR of the ForceFreeStates modularization sequence (see
REFACTOR_PLAN.md, §5–§7). Stacked on #381/#387; auto-retargets todevelopwhen those merge.Status: implementation complete — all gates green
Landed:
Commit (a) — result struct (§5): every solve publishes an immutable
ForceFreeStatesResult; presence-equals-capability optional fields (solution::SolutionProfiles,free_boundary,delta_prime,galerkin,diagnostics); universalclosure/bpen; fixed-boundarywp; standaloneintegrator = "galerkin"(additive gal removed,gal_flagdeprecated); consumers (PE, writer, SLAYER, IMAS) re-signatured onto the result with warn-and-skip gates.Commit (b) — staged main (§6):
main_from_inputsdecomposed into stage functions (resolve_mode_space!→ … →run_slayer_stage); pure motion, verified by normalized diff and byte-identical HDF5 output.Reconciliation merge: ALL - IMPROVEMENT - Self-describing HDF5 metadata (issue #226, PR B) #364's literature dataset renames grafted onto the re-signatured writer; metadata calls re-scoped;
dVdpsi_splinekwarg threaded through.Commit (b2) — unified Δ′ payload (§6A, D14): one
delta_primetype (matrix, raw D′,delta_coil, PEST-3 blocks) produced by riccati AND galerkin on shared canonicalSingularSurfaces/paths; gal-fed SLAYER enabled; found and fixed a pre-existing bug where the galDelta_prime_rawdataset carried duplicatedDelta_coilrows.Commit (c) — scripting API (§7): CommonSolve entry point —
EulerLagrangeProblem(eq; nn, …)names the problem,solve(prob, Forward()/Riccati()/Galerkin())picks the formalism (solve(eq, alg; …)sugar retained);PlasmaEquilibrium(path; …); abstractRMPFieldwith lazy source algebra (+,-, scalar*— weights are linear-combination coefficients, not physical amplitudes); purematerialize_forcing_modes((ffs, source) -> (spectrum, coil_geometry), forcing state owned solely bycompute_perturbed_equilibrium);perturbed_equilibrium(ffs, rmp)with the TOML driver routed through the same single forcing path; Scripting API docs page.Commit (d) — cross-formalism file contract (§7A; gpec.h5 schema: follow-ups from the #364 standardization audit #388 items 1+2): closed ξ profiles written from
result.solutioninto the producing formalism's Solutions group with identical names and(mode, solution, psi)axis order (GalerkinIntegration/{psi,q,xi_psi,dxi_psidpsi,xi_s}joins the untouched Forward layout;Match/xiremoved — it was the profiles mislabeled as diagnostics); raw gal outer basis demoted to a debug output (Basis/, gated byDebugSettings.gal_basis_output);Tearing/PerSurface/rational_psi/rational_qso gal-fed SLAYER output identifies its surface subset. Remaining gpec.h5 schema: follow-ups from the #364 standardization audit #388 items stay on the issue.Verification
result.solutionexactly.gal_resistive_diiid— 3 "changed" rows are a cross-formalism read artifact: the additive develop deck writes the riccati BVP's Δ′ at the canonical names, the branch writes the gal Δ′ there (values verified bit-identical gal-vs-gal in a dedicated dual-run). Re-baselined.gal_resistive_pe— 8 PE quantities missing on the branch: galerkin-fed PerturbedEquilibrium warn-skips until the gal free-boundary δW lands (next cycle); the develop values came from the removed additive-gal path.Verification so far: 82/82 result-struct tests, 357/357 across six targeted files, forward byte-identity and gal-group equivalence vs the pre-merge base; post-merge smoke 82/82 + 66/66 (slayer). Full suite + docs + regression-harness report will be posted before this leaves draft.
Planned follow-up PRs (deliberately NOT in this PR — full specs in
REFACTOR_PLAN.md§7B)This PR establishes the API surface; two follow-on PRs complete the campaign, in this order:
main()becomes a deck interpreter: parsegpec.tomlinto the same objects and calls a script makes (EulerLagrangeProblem+ integrator →solve);main_from_inputsand the stage functions dissolve. The deck is the API, serialized — one path to maintain.PlasmaEquilibrium(the two-pass grid refinement needs them at equilibrium formation), unlockingkinetic_factor > 0throughsolve. Coordinates with GPEC - IMPROVEMENT - Make all input control structs immutable #367 (struct freeze) — lands after it.gpec.h5is replayable and h5→toml regeneration falls out; closes the current gap where API-run outputs carry no input blob.GeneralPE = perturbed_equilibrium(ffs)builds the source-independent response/coupling operators once;force(GeneralPE, fields)materializes sources, applies P, and computes derived quantities. Enables cheap coil-current scans/optimization over one response operator, and pairs with the Δ_mn resonant-coupling work. Two binding requirements already recorded:resp_index): the Julia port implements only the energy method (resp_index=0, wt0-based); the four surface-current methods (kapmatsvariants) were never ported. A typedResponseMethodselection restores them — and the surface-current port is what unlocks galerkin-fed PE independently of the gal-δW work (in Fortran,gal_flagforcesresp_index=1).RMPFieldleaf kind ever — a spectrum-literal — with the Make surface input always at psi=1 or on a specifiable surface, then use method of equivalent surface currents to get the currents on the control surface #377 equivalent-surface-currents solve as a utility, not a type; external codes couple by producing control-surface spectra.Also queued independently: gal free-boundary δW (fills
wp/free_boundaryfor Galerkin, method-0 parity), Δ_mn fromdelta_coilasymptotics for Riccati+Galerkin resonant coupling, the remaining #388 items, and the ForceFreeStates directory reorg (#394, after #367 and the formatter PR).Capability matrix (target, D14 — full details in
REFACTOR_PLAN.md§9)Outputs fall into three physics classes: control-surface quantities (plasma boundary — every integrator), class 1 full in-plasma ξ/ξ′ profiles (spectral, full-volume perturbed equilibria), and class 2 rational-surface resonant data quantifying island-opening drive (
bpen, and the futuredelta_mn— the pitch-resonant derivative jump at each rational surface, yielding perturbed current and shielded resonant flux).Legend: ✅ implemented · 🔜 pending the named follow-on work · ❌ never · — N/A.
wp(control surface)free_boundaryenergies (control surface)solution— full ξ/ξ′ profiles (class 1):el_axis:gal_nativeclosure/bpen(class 2; always present, zeros under:ideal):ideal:ideal(🔜:matchedwith STRIDE matching):idealor:matcheddelta_mn(class 2; resonant-derivative jump)delta_coildelta_coildelta_prime— ONE unified type: Δ′ matrix, raw D′,delta_coil, PEST-3 blockspest3_decompose)diagnostics: crit, nzero, edge scan, ca)kinetic_factor>0)delta_primeThis PR must not be merged into
developuntil a human reviewer has approved it. This requirement is absolute.🤖 Generated with Claude Code