Skip to content

Repo - REFACTOR! - Rename TOML config variables to descriptive spellings - #382

Open
priyanshlunia wants to merge 1 commit into
developfrom
docs/toml_variable_cleanup
Open

Repo - REFACTOR! - Rename TOML config variables to descriptive spellings#382
priyanshlunia wants to merge 1 commit into
developfrom
docs/toml_variable_cleanup

Conversation

@priyanshlunia

@priyanshlunia priyanshlunia commented Aug 14, 2026 •
edited by logan-nc
Loading

Copy link
Copy Markdown
Collaborator

Release note

  • Audience: users
  • Numerical impact: none; only input names change. (harness @ 008edd9)
  • Migration: rename these gpec.toml keys and values. The old spellings are not accepted.
Section Old New
[Equilibrium] etol, r0exp, b0exp flux_surface_rtol, r0_norm, b0_norm
[Equilibrium] grid_type "ldp" "core_edge_packed"
[ForceFreeStates] dmlim, set_psilim_via_dmlim rational_offset_fraction, truncate_at_rational_offset
[ForceFreeStates] psiedge, ucrit dW_edge_scan_start, renorm_threshold
[ForceFreeStates] mthvac, nzvac, sing_order mtheta_vacuum, nzeta_vacuum, frobenius_order
[PerturbedEquilibrium] reg_spot regularization_width
[KineticForces] zi, mi, zimp, mimp ion_charge, ion_mass, impurity_charge, impurity_mass
[KineticForces] nl, atol_xlmda, rtol_xlmda bounce_harmonic_max, atol_pitch, rtol_pitch
[KineticForces] wdfac, nufac, divxfac magnetic_drift_factor, collisionality_factor, divxi_factor
[KineticForces] f0type "jkp" "park"
[SLAYER] dc_type, dr_val, dgeo_val, mu_i D_c_type, D_R, D_geo (as in the SLAYER HDF5 outputs), ion_mass
[SLAYER] D_c_type "rfitzp" "fitzpatrick"
[Wall] a standoff_fraction (conformal, dee) or minor_radius (elliptical, mod_dee)
[Wall] aw, bw, cw, dw, tw half_thickness, elongation, center_offset, triangularity, corner_sharpness
[SOL_INPUT] e elongation
[LAR_INPUT], [TJ_ANALYTIC_INPUT] lar_r0, lar_a, mtau r0, a, mtheta
[TJ_ANALYTIC_INPUT] qc, pc, mu q0, p0, pressure_peaking
[[ForcingTerms.coil_set]] xnom, ynom, znom rotation_center_x, _y, _z
[[ForcingTerms.coil_set]] ncoil_gen, nsec_gen n_coils (shared with helical), points_per_coil

use_galgrid, nstep, diagnose_ca, diagnose and ksing were never read or only half-implemented; they join the existing deprecated-key lists, so they are dropped with a warning. Unknown [[ForcingTerms.coil_set]] keys are now an error instead of being ignored.

Closes #287.

What changed

  • Rename only. No release has been made yet, so there is no back-compat remap.
  • Guessable Fortran-heritage names are kept (psilow/psihigh, qlow/qhigh, mpsi, mtheta, newq0, singfac_min).
  • Internal physics code keeps its short names (inner-layer dc_type/dr_val, the Wall shape formulas, KineticForces kernels); only the user-facing config fields change.
  • Not in this PR: the 16 KineticForces method flags (fgar_flag …), which need a redesign rather than renames, and the gal_* Galerkin knobs.

Regression report

Run at 4e6626a30 (008edd9bc only fixes two test lines after it), 8 cases, develop (ea8bfc960) vs branch:

solovev_n1                  Summary: 22 unchanged
diiid_n1                    Summary: 53 unchanged
diiid_n1_riccati            Summary: 17 unchanged
diiid_slayer_n1             Summary: 16 unchanged, 1 flagged (noise, below)
solovev_kinetic_calculated  Summary: 17 unchanged
solovev_kinetic_ntv         Summary:  6 unchanged
solovev_multi_n             Summary: 15 unchanged
gal_resistive_diiid         Summary: 10 unchanged

The flagged SLAYER γ_Hz is run-to-run noise in develop itself: two runs of ea8bfc960 gave 191.711 and 191.478 Hz for the 2/1 root, and the branch runs land on one of those two values. Full test suite passes at 008edd9bc.

Redo of the original branch (old head cab894a00) on current develop. Develop had already replaced use_parallel/use_riccati with its own integrator enum, so those rows are gone.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PSrf6JCViFfVzqzkQ66o6b

@priyanshlunia priyanshlunia added architecture refactor Behavior-preserving restructuring labels Aug 14, 2026
logan-nc added a commit that referenced this pull request Aug 17, 2026
@logan-nc @claude
EQUIL/EXAMPLES - Default psihigh_from_layer_overlap on across every deck
3cea719 
Flips EquilibriumConfig.psihigh_from_layer_overlap to true and sets it
explicitly, annotated, in all 17 decks. Kinetic profiles are wired via a
[SLAYER] profile_file entry using each deck's OWN family file -- never a
[KineticForces] block, which would flip needs_kinetic_profiles and change the
auto-grid knot density, an unrelated results channel. A [SLAYER] section
carrying only profile_file is inert: slayer_ctrl.enabled defaults false and the
stage returns before running or writing anything.

Deck sweep, equilibrium stage, all 17:
  MOVED  DIIID-like_SLAYER          0.9995 -> 0.993281
  MOVED  DIIID-like_ideal           0.995  -> 0.993281
  no-op  DIIID gal_resistive, _pe   0.993 (already at the floor)
  no-op  Solovev x5, LAR x4, a10
  n/a    DIIID-like_ideal_IMAS      needs a runtime dd; flag left off anyway

The analytic decks no-op exactly as predicted: limited plasmas with finite edge
q, so the edge q-law fit is rejected and the two well-separated surfaces never
overlap. LAR has no legitimate kinetic file, so those four no-op on the missing
profiles instead.

NOTE FOR REVIEW: on all four DIII-D decks the psiedge floor OVERRIDES the layer
criterion. Each asks for ~0.9892 and each is held at the q=6 window top. The
feature as configured therefore reduces to "clamp psihigh to the first rational
surface outboard of psiedge"; the layer physics sets the domain nowhere in the
shipped examples.

IMAS deck keeps the flag off with a comment: its rerun path re-runs read_imas and
must resolve the same psihigh on both passes (_validate_psi_nodes errors
otherwise), which is what resolving a new domain changes; "imas" is also outside
EFIT_KINDS so there is no separatrix clamp and no raw psi map.

benchmarks/equil_psihigh_scan.jl now records the structured equilibrium_edge_quality
verdict per point instead of a hand-rolled round-trip, captures et/ep/ev, and adds
a gate-validation pass that fails loudly if any psihigh exists where the gate
passes while the energies are already unusable. It forces the flag off so the
ladder tests the psihigh it sets.

docs: corrects a naming bug I introduced -- workflow.md called the variable
dW_edge_scan_start, but that rename is an unmerged draft (PR #382) and the code
says psiedge. Documents the floor, the cross-section dependency on
[ForceFreeStates] psiedge, and that truncations are reported in mm as well as psi.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PSrf6JCViFfVzqzkQ66o6b
@logan-nc logan-nc mentioned this pull request Aug 19, 2026
Merged
@logan-nc

logan-nc commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

@priyanshlunia #363 and #364 were merged. Do you want to mark this as ready for review?

It now has a lot of conflicts, but I suspect they are largely trivial. Please merge in the newest develop and ping us afterwards when it is ready for review. Is the assignee yourself?

@logan-nc @claude
Repo - REFACTOR! - Rename TOML config variables to descriptive spellings
008edd9 
Keys and values whose names a reader cannot guess are renamed; guessable
Fortran names (psilow, qhigh, mpsi, newq0, ...) are kept.

- [Equilibrium] etol, r0exp, b0exp -> flux_surface_rtol, r0_norm, b0_norm; grid_type "ldp" -> "core_edge_packed"
- [ForceFreeStates] psiedge -> dW_edge_scan_start; dmlim, set_psilim_via_dmlim -> rational_offset_fraction,
  truncate_at_rational_offset; ucrit -> renorm_threshold; mthvac, nzvac -> mtheta_vacuum, nzeta_vacuum;
  sing_order -> frobenius_order
- [PerturbedEquilibrium] reg_spot -> regularization_width
- [KineticForces] zi, mi, zimp, mimp -> ion_charge, ion_mass, impurity_charge, impurity_mass; nl -> bounce_harmonic_max;
  wdfac, nufac, divxfac -> magnetic_drift_factor, collisionality_factor, divxi_factor; *_xlmda -> *_pitch; f0type "jkp" -> "park"
- [SLAYER] dc_type, dr_val, dgeo_val -> D_c_type, D_R, D_geo (as in the SLAYER HDF5 outputs); mu_i -> ion_mass; "rfitzp" -> "fitzpatrick"
- [Wall] a splits into standoff_fraction (conformal, dee) and minor_radius (elliptical, mod_dee); aw, bw, cw, dw, tw ->
  half_thickness, elongation, center_offset, triangularity, corner_sharpness
- [SOL_INPUT] e -> elongation; [LAR_INPUT]/[TJ_ANALYTIC_INPUT] lar_r0, lar_a, mtau -> r0, a, mtheta; TJ qc, pc, mu -> q0, p0, pressure_peaking
- [[ForcingTerms.coil_set]] xnom/ynom/znom -> rotation_center_x/_y/_z; ncoil_gen merges into n_coils; nsec_gen -> points_per_coil;
  unknown coil_set keys are now an error
- Unimplemented use_galgrid, nstep, diagnose_ca, diagnose and ksing join the existing deprecated-key lists

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PSrf6JCViFfVzqzkQ66o6b
@logan-nc
logan-nc force-pushed the docs/toml_variable_cleanup branch from cab894a to 008edd9 Compare October 4, 2026 03:33
@logan-nc logan-nc self-assigned this Oct 4, 2026
@logan-nc logan-nc changed the title DOCS/CONFIG - REFACTOR - Rename TOML config variables to descriptive spellings (closes #287) Repo - REFACTOR! - Rename TOML config variables to descriptive spellings Oct 4, 2026
@logan-nc
logan-nc marked this pull request as ready for review October 4, 2026 03:34
@github-actions github-actions Bot added the changed-results Results move or an interface breaks - read before upgrading label Oct 4, 2026
@logan-nc
logan-nc removed their request for review October 4, 2026 03:34
@logan-nc

logan-nc commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator

@priyanshlunia I rebuilt this branch on current develop and force-pushed it (your old head is cab894a00):

  • Develop already has its own integrator enum, so those rows are gone.
  • Rename only, with no remap for old keys, since there's no release yet.
  • Wider pass per TOML variable cleanup #287: dmlim, ucrit, mthvac, the Wall shape params, the KineticForces species and scaling keys, and more. The full table is in the body.
  • ldp is now core_edge_packed. It's a sin² grid that is dense at the axis and edge, not rational-packed.

@logan-nc

logan-nc commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator

@d-burg @jhalpern30 @matt-pharr are you guys OK with doing this name change sweep now?

@logan-nc logan-nc added this to the GPEC v2.0.0 milestone Oct 6, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@d-burg d-burg Awaiting requested review from d-burg

At least 1 approving review is required to merge this pull request.

Assignees

@logan-nc logan-nc

Labels

changed-results Results move or an interface breaks - read before upgrading refactor Behavior-preserving restructuring

Projects

None yet

Milestone

GPEC v2.0.0

Development

Successfully merging this pull request may close these issues.

TOML variable cleanup

2 participants

@priyanshlunia @logan-nc