Repository navigation
PerturbedEquilibrium - API! - Build the plasma response once and force it per source - #574
Conversation
github-actions
Bot
added
api
…keys' into refactor/plasma-response
…the forcing normalization docstring
…keys' into refactor/plasma-response
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0186CLfjeJeSusNTva5RMfGU
logan-nc
left a comment
logan-nc
left a comment
There was a problem hiding this comment.
Read in full. One real bug, one stale example, and two behaviour changes to disclose; then ready to approve.
benchmarks/benchmark_diiid_ideal_ntv_torque.jl:266breaks: it buildsPerturbedEquilibriumState(; psi_grid, xi_modes)andresponse::PlasmaResponseis now a required field (UndefKeywordError). Nothing tests it. Needs a stub response (or a constructor that takesffs).- Stale docstring example
src/ForcingTerms/ForcingTerms.jl:292:pe = perturbed_equilibrium(ffs, total)no longer dispatches; should readperturbed_equilibrium(plasma_response(ffs), total). Not pushed (src, stamp). efc_couplingsregularization is a silent numerical change. On develop it built its ownPerturbedEquilibriumControland therefore always used the defaultregularization_width = 0.05, whatever the deck said; now it takes the run's width (0 on a kinetic fit). That is the right behaviour, but any error-field deck with a non-default width, or any kinetic fit, moves;diiid_error_fieldsits at 0.05 so the harness cannot see it. One line in the release note, and arguably the!already on the title covers it.- Galerkin/matched results now error unconditionally at
plasma_response, deleting the "both flags off" path and test #556 just added. Say in the Migration line that this supersedes #556's advice (the example moves toforce_termination).
Also: no test that an explicit regularization_width is respected on a kinetic result (the release-note behaviour); docs/src/api.md "Entry points" does not list plasma_response/PlasmaResponse; docs/src/workflow.md Stage 5 still describes one stage. Stale comment in test/runtests_dominant_coupling.jl ~90 (the applied vector is no longer evaluated by the writer).
Pushed 85cf562 (docs only): kinetic_forces.md still said the width is forced to 0 and logged; it is now a default.
Checked and clean: the TOML key split at the driver matches the two signatures; no stale accessors of the moved operators anywhere (Analysis, Rerun, writers read HDF5 paths; KineticForces reads only xi_modes/psi_grid); every PerturbedEquilibriumControl/Internal field is accounted for; compute_dominant_coupling! → build_dominant_coupling! has no leftovers; the solve-API tests exercise the two-call path substantively (reuse, scale, algebra, spectrum leaf).
Collisions (git merge-tree): vs develop 3 files (hdf5-conventions.md, PerturbedEquilibrium.jl, PerturbedEquilibriumStructs.jl); vs #580 the efc_couplings call in the driver; vs the KineticForces stack and #588 the same files as #569. The harness stamp is d09bf3c under a later merge commit; CI accepts it.
|
Heads-up: GitHub did not start the pull-request workflows on 85cf562 (my push above); no check runs exist for it after an hour although Actions itself is fine. A close/reopen or your next push will re-fire them. The change is non- |
…keys' into refactor/plasma-response
…o 0 for every result
…keys' into refactor/plasma-response
|
Thanks, all addressed in 60ed66b except the kinetic-width test, which is in the next commit. Taking your items in order:
The smaller ones:
Develop's #472 landed in between, so the torque response is ported onto the split and stays opt-in. Restamped at 60ed66b against #569's 58381b6, all 19 cases: nothing moves (263 quantities exactly 0, including |
… explicit width on a kinetic result
…keys' into refactor/plasma-response
…keys' into refactor/plasma-response
logan-nc
left a comment
logan-nc
left a comment
There was a problem hiding this comment.
Re-read 60ed66b and 5e49810. Every item from the first review is in: the benchmark passes a PlasmaResponse(; ffs) stub, the ForcingTerms docstring example is the two-call form, the width change in efc_couplings and the end of the #556 fallback are both in the Migration line, api.md links plasma_response/PlasmaResponse, workflow.md Stage 5 is two steps, the stale comment is fixed.
On the new default of 0 for every result: I checked the decks. The only kinetic-solve example (Solovev_kinetic_calculated, kinetic_factor = 1) leaves the width unset, so it stays at 0 as it was when the code forced it; the four decks that drive KineticForces from an ideal solve keep 0.05 explicitly; the three decks that went to 0.0 are ideal runs whose regularized profiles are not tracked. That matches the Numerical impact line. The kinetic-width test in runtests_torque_response.jl pins the explicit-value path. Ready from my side; lands after #569 and retargets to develop then.
…keys' into refactor/plasma-response
…keys' into refactor/plasma-response
Release note
regularization_widthnow defaults to 0, so ideal runs that leave it unset get unregularized profiles near the rationals; error-field NTV couplings now use the run's width. No tracked harness quantity moves (harness @ f684954)perturbed_equilibrium(ffs, rmp; ...)is replaced by two calls,resp = plasma_response(ffs; compute_singular_coupling=true, compute_torque_response=false)thenperturbed_equilibrium(resp, rmp; regularization_width, verbose, write_outputs_to_HDF5, output_filename).PerturbedEquilibriumControlandPerturbedEquilibriumInternalare gone. The response and coupling matrices, rational-surface labels and dominant modes moved from the result to itsresponsefield (pe.response.permeability,pe.response.C_delta_prime, ...).ResonantCoupling(pe, ffs)is nowResonantCoupling(pe.response). A deck that setscompute_responsein[PerturbedEquilibrium]now stops; delete the line.compute_torque_responsestill lives in that section and now goes toplasma_response;compute_torque_response!andtorque_response_matricesare replaced bytorque_response_matrix,build_torque_response!andapply_torque_response!.regularization_widthnow defaults to 0 for every result (Fortranreg_flag = f); before, it defaulted to 0.05 on ideal results and was forced to 0 on kinetic ones. Only the regularized profiles move (dxi_clebsch_psidpsi,xi_clebsch_alpha, contravariant ξ^θ/ξ^ζ, the*_regfield and current datasets, and the KineticForces torque built from them); response and coupling matrices, resonant fields, Δ′, islands and energies do not. A deck that drives KineticForces with an ideal solve should setregularization_width = 0.05, as the example decks that do so already do. An explicit width is now respected on kinetic results too.efc_couplingsnow uses the run's width instead of always 0.05, so error-field NTV couplings move on decks whose width is not 0.05. Galerkin and:matchedresults now error atplasma_responsewhatever the flags, which supersedes PerturbedEquilibrium - BUGFIX! - Refuse Galerkin and matched results until they publish wp #556's advice to turncompute_responseandcompute_singular_couplingoff; stop such a run after force-free states instead.The perturbed-equilibrium stage used to rebuild the plasma response and singular-coupling operators for every forcing source.
plasma_response(ffs)now builds them once into aPlasmaResponse, andperturbed_equilibrium(resp, rmp)applies them to one field, so scans over sources and the error-field correction couplings reuse one response.RMPField(modes)wraps a control-surface spectrum computed elsewhere as a forcing source.Regression report
One SDCC fan-out over all 20 harness cases at 8 CPUs per task (Julia 1.12.6 with the CI manifest pin) tracked develop (db90e82), #569 (7fd923d) and #574 (125f4a0) in a single run. Checked against that run's results database rather than only the report's last-vs-previous summary, every stored value other than runtime is bit-identical from develop to #569 and from #569 to #574: 271 tracked values in 19 cases per step, integers, reals, full arrays and checksums alike.
gal_resistive_pereads its 8 expected N/A on all three refs, since develop refuses that Galerkin PE run too.The stamp is f684954, which adds only the merge of #569's test commit (5786d2d:
PlasmaEquilibriumin three tests, and a fixed@test_brokenpromoted) on top of 125f4a0.The regularization default cannot show here: the two cases whose decks moved to 0 (
solovev_n1,diiid_multi_n) track no regularized quantity, and every case that does keeps an explicit 0.05.TorqueResponse/total_torqueinsolovev_kinetic_calculatedruns the torque response through its new home.Full report (tracking all three refs)
Notes for reviewers
Stacked on #569. The harness tracks develop, #569 and this branch in one run.
The split. Everything that depends only on the force-free-states result moves into
plasma_response:C_coeffs, rational metadata);perturbed_equilibrium(resp, rmp)keeps everything per source: materializing the field, P·Φₓ, the three spectra, the energies, the profile reconstruction, the applied resonant vectors, the island widths and Chirikov, and the dominant overlap Vᴴ·b̃ₓ.Response.jlandSingularCoupling.jlare split along that line without changing the arithmetic. It still writes gpec.h5 itself, as before; moving the write is the consolidation's.What else changes.
efc_couplingstakes the run'sPlasmaResponseand the run'sregularization_width, and forcesRMPField(modes). Before, it did two full PE solves per correction coil set, rebuilding Λ, L, P and the flux matrix each time with a hard-coded default control, so it always regularized at 0.05 whatever the run used.diiid_error_fieldsets 0.05, so the harness cannot see the fix.regularization_widthdefaults to 0 for every result, matching the Fortranreg_flag = ftemplate;default_regularization_widthis gone. Regularization exists to converge the NTV torque of an ideal solve, so the decks that drive KineticForces with one (DIIID-like_ideal,DIIID-like_error_field,Solovev_kinetic_NTV,Solovev_kinetic_multiion) keep 0.05, andSolovev_ideal,DIIID-like_ideal_multi_nandDIIID-like_gal_resistive_pego to 0.plasma_response(ffs; compute_torque_response=true)buildsT_xe(ψ)ontoPlasmaResponse.T_xe(the ψ loop and its checks are unchanged), and eachperturbed_equilibriumcall contracts it with its own forcing intoT_appliedandT_source = B̃†·T_xe·B̃withB̃ = R⁻¹·Φ_source, which equals PerturbedEquilibrium - FEATURE - Add the psi-resolved torque response matrices T_xe and T_source #472's per-source form algebraically.runtimeskeyword is gone from both calls, as develop removed it frommain.[PerturbedEquilibrium]section is split by key between the two calls.compute_responsejoins the deprecated-key list and the hook, and is removed from the example decks and tests. The inertregularization_widthline goes fromSolovev_kinetic_calculated, the only kinetic example.PerturbedEquilibriumInternalcarried.plasma_responseand is unconditional. The Galerkin PE example keeps its[PerturbedEquilibrium]section and now stops there, with its stagedcompute_singular_coupling = truerestored; a follow-up makes it run. Thegal_resistive_peharness case ends after force-free states ([overrides] ForceFreeStates.force_termination = true), with a comment that the follow-up removes it.Bits. Each version is exactly deterministic run to run. Between them, Equilibrium and ForceFreeStates are identical, and PerturbedEquilibrium agrees to rounding: the forcing amplitude differs by 1 ulp, most datasets by about 1e-14, at worst 1.8e-10 relative in
Jb_zetaon the coarse Solovev fixture. In one process, materializing the forcing before or after building the response gives identical bits, so the seed comes from the reorganized flow itself, plausibly allocation-dependent vectorized reductions (not isolated). None of this reaches a tracked harness quantity (below).Tests.
runtests_solve_api.jlnow checks that one response forced twice is identical; thatscaleand3A − Abehave linearly on the reused response; thatRMPField(pe.forcing_modes)reproduces the file-forced run exactly, takes a weight and sums with a file leaf; that the default width is 0 and an explicit 0.05 is respected; and that the deck and the two calls agree on hdf5 file forcing.plasma_response. The island diagnostics are a pure function and tested as one.runtests_torque_response.jldrives the new two-step flow, checks a forcing file as one source and that a response built without the option contracts nothing, and checks that an explicit width is respected on its kinetic result.@test_broken), kinetic validity 21.🤖 Generated with Claude Code