VAC - REFACTOR - Give the Vacuum module its own output struct - #358
Open
jhalpern30 wants to merge 7 commits into
Open
VAC - REFACTOR - Give the Vacuum module its own output struct#358jhalpern30 wants to merge 7 commits into
jhalpern30 wants to merge 7 commits into
Conversation
… struct mutation First installment against #139: make it clear from a call site what a function changes, by shrinking what the shared mutable structs carry and narrowing who is allowed to write to them. Further struct cleanups will land on this branch. Freeze ForceFreeStatesControl: - ForceFreeStatesControl is now immutable, so ctrl can never appear as the first argument of a ! function. ctrl.nn_low/nn_high keep what the user asked for; the resolved toroidal range lives on intr.nlow/nhigh, and the Fortran DCON delta_mhigh doubling is applied where it is consumed rather than written back onto ctrl 130 lines earlier. - The Riccati dense-xi pass no longer clobbers and restores three ctrl flags around a nested integration. The serial branch of eulerlagrange_integration is extracted as serial_eulerlagrange_integration and called directly, which also removes a mutation of shared state from a function reachable on the threaded path. - sing_lim! normalizes dmlim into a local instead of writing it back to ctrl. Return values instead of parking them on intr: - Local stability is returned by the new compute_local_stability rather than stored on intr.locstab. The field is removed, along with the zeros-filled spline that was built on every run even with local_stability_flag = false; write_outputs_to_HDF5 takes locstab as a keyword. Docstrings: - eulerlagrange_integration and parallel_eulerlagrange_integration both documented "-> OdeState" while returning a 4-tuple. Corrected, and dropped stale references to euler.h5 and to setting ctrl fields programmatically. No numerical change is intended: every edit is a data-flow refactor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
VacuumData lived in ForceFreeStates and was passed into Vacuum as an untyped output buffer, so the Vacuum module could not name the type it wrote to. Its 14 fields mixed vacuum outputs, free-boundary energies computed in Free.jl, and sizing scratch. Split into two focused immutable structs: - Vacuum.VacuumResponse (src/Vacuum/DataTypes.jl) holds what the Vacuum module computes: wv, grri, grre, plasma_pts, wall_pts, mtheta, nzeta. compute_vacuum_response returns it instead of a positional 5-tuple that every caller partly discarded, and compute_vacuum_response! is typed rather than duck-typed on field names. - ForceFreeStates.FreeBoundaryResult holds the energy decomposition, built once at the end of free_run instead of being mutated in place across 40 lines. grri/grre are no longer retained past free_run: nothing read them there, and downstream consumers recompute their own. Drops ~2 MB of live allocation per run at mthvac=512. The dead numpoints/numpert_total/mthvac fields are gone too. Consumers narrowed to what they actually read (issue #139 principle 1): galerkin_solve takes wv, compute_perturbed_equilibrium takes wt0 and mthvac and no longer imports a ForceFreeStates struct at all, and build_flux_matrix loses a parameter it never used. No intended numerical change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Conflicts resolved by combining both sides: - ForceFreeStatesStructs.jl / Riccati.jl: develop's du_store / xi_s_store store renames applied to this branch's condensed docstrings and free_run rename. - Free.jl: normalize_eigenfunctions! docstring keeps its own description with develop's accurate store list. - PerturbedEquilibrium.jl: develop's new ffit argument to compute_singular_coupling_metrics! combined with this branch's vac_data -> mthvac narrowing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
Author
|
As part of this, I also extracted |
…ting a ctrl copy
Freezing ForceFreeStatesControl left one consumer behind: galerkin_solve built
ctrl_gal = deepcopy(ctrl) and assigned ctrl_gal.sing_order per surface, which
throws on an immutable struct. Any run with gal_flag = true and at least one
singular surface failed with
setfield!: immutable struct of type ForceFreeStatesControl cannot be changed
No unit suite covers the Galerkin path, so this only surfaced under the
regression harness.
compute_sing_asymptotics and compute_sing_mmat! now take a sing_order keyword
defaulting to ctrl.sing_order, and galerkin_solve passes the per-surface value
directly, so the deepcopy is gone. As a side effect SingAsymptotics now records
the order actually used rather than whatever was last written onto the copy.
No numerical change: diiid_n1 48/48, gal_resistive_diiid 10/10 and
gal_resistive_pe 4/4 unchanged against a0cad26.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…um-response-struct
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
1 participant
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.
Follow-on to #355 (branched off it, so this diff also contains #355's commits until that merges).
What
VacuumDatalived inForceFreeStatesand was passed into the Vacuum module as an untyped output buffer —Vacuum.jlsaid so outright: "designed to work withForceFreeStates.VacuumDatabut does not depend on its concrete type (duck-typed on field names only)." Vacuum sits below ForceFreeStates in the dependency graph, so it could not name the type it wrote to. Its 14 fields mixed three unrelated groups: vacuum outputs, free-boundary energies computed inFree.jl, and sizing scratch.Split into two focused immutable structs:
Vacuum.VacuumResponse(src/Vacuum/DataTypes.jl) — what the Vacuum module actually computes:wv, grri, grre, plasma_pts, wall_pts, mtheta, nzeta.compute_vacuum_responsereturns it instead of a positional 5-tuple that every caller partly discarded (wv, _, _, _, _/_, grri, grre, _, _), andcompute_vacuum_response!is now typed rather than duck-typed on field names.ForceFreeStates.FreeBoundaryResult— the energy decomposition, constructed once at the end offree_runinstead of being mutated in place across 40 lines.Dead weight removed
grri/grrewere write-only on this path: Vacuum filled them, nothing ever readvac_data.grri/.grre, andResponse.jl/SingularCoupling.jleach callcompute_vacuum_responseagain with their own inputs. They are no longer retained pastfree_run— roughly 2 MB of live allocation per run atmthvac=512,mpert≈65.numpoints,numpert_total, andmthvachad zero readers and are gone.Consumers narrowed (issue #139 principle 1)
galerkin_solveread only.wv→ takeswv.compute_perturbed_equilibriumread only.wt0and.mthvac→ takes those two, and no longer imports a ForceFreeStates struct at all.build_flux_matrixtook avac_datait never used → parameter dropped.Verification
No intended numerical change — no arithmetic was moved.
Regression harness, run against
a0cad260(the exactdevelopcommit merged here, so the comparison isolates only this branch's changes):diiid_n1gal_resistive_diiidgal_resistive_pesolovev_n1solovev_multi_nSuites:
runtests_vacuum.jl273/273 ·runtests_fullruns.jl17/17 ·runtests_sing.jl75/75 ·runtests_imas.jl24/24 ·runtests_resist_eval.jl63/63 ·runtests_coordinate_invariant.jl·runtests_rerun_from_h5.jl.Notes for the reviewer
compute_vacuum_responsereturns a struct rather than a 5-tuple. Nothing in-repo outside the tests calls it directly.developresolved by combining both sides (develop'sdu_store/xi_s_storerenames and its newffitargument, with this branch's docstring condensation andvac_data→mthvacnarrowing).90f2bb34and inherited here by merge: freezingForceFreeStatesControlleftgalerkin_solvedoingdeepcopy(ctrl)+ctrl_gal.sing_order = ..., which throws on an immutable struct. Everygal_flag = truerun was failing. No unit suite covers the Galerkin path, which is why it stayed hidden.grri/grreVacuum-internal and exposesI_vinstead, and it touches nearly the same file set. It conflicts hard here — both rewrite_compute_vacuum_response_2d!and thecompute_vacuum_responsereturn convention, and VACUUM - Migrate surface current calculation into the VACUUM module #345 deletescompute_surface_inductance_from_greensoutright. Suggest landing VACUUM - Migrate surface current calculation into the VACUUM module #345 first; this branch'sVacuum.jldiff then collapses to wrapping its result in the struct, withI_vbest expressed asUnion{Matrix{ComplexF64},Nothing}rather than a zeros matrix.🤖 Generated with Claude Code