Skip to content

Equilibrium - BUGFIX - Build the two-pass ψ grid from kinetic profiles that have no omega_E - #617

Merged
logan-nc merged 5 commits into
developfrom
bugfix/two-pass-grid-without-omega-e
Nov 2, 2026
Merged

logan-nc merged 5 commits into
developfrom
bugfix/two-pass-grid-without-omega-e

Conversation

@d-burg

@d-burg d-burg commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Release note

  • Audience: users
  • Numerical impact: none expected on the shipped decks, whose kinetic files all carry omega_E. A deck whose kinetic profiles lack omega_E now runs: its two-pass grid follows the equilibrium alone, where before this it stopped in the equilibrium stage (harness @ 4bba6e7)
  • Migration: none

A run whose kinetic file has no omega_E no longer stops in the equilibrium stage. SLAYER growth rates can again be computed from such files, assuming Ω_E = 0 with a warning, as before kinetic profiles moved onto the equilibrium. Runs with [KineticForces] or kinetic_source = "calculated" still need omega_E, and now stop up front, before any solve, saying so.

Closes #614

Regression report

After the port onto the v2 layout: ITER SDCC fan-out, develop @ e51f4b0ee vs bugfix/two-pass-grid-without-omega-e @ 4bba6e7a5, all 26 cases. Every case is unchanged, all 25; gal_resistive_pe is N/A on both refs, by design.

regress --refs 728a7e7d5,5f7d413ce --force (all cases) after merging develop, on feynman (SLURM, 8 threads, Julia 1.11.9, manifest pinned): every case is unchanged, all 24; gal_resistive_pe is N/A on both refs, as on develop. runtests_solve_api.jl (152) and runtests_grid_refinement.jl (84) pass at 5f7d413ce.

Before that merge, regress --refs 08a6975ef,fc9d9bb74 --force (all cases) on feynman: every case is unchanged, all 24 including the kinetic, SLAYER and lar_reversed_shear_n1 cases; gal_resistive_pe is N/A on both refs, as on develop. runtests_solve_api.jl passes 152/152 at fc9d9bb74.

An earlier run, regress --refs 4f33fcf60,c05b2babc --force, was also unchanged. On feynman at c05b2babc, runtests_solve_api.jl (148), runtests_grid_refinement.jl (84), runtests_kinetic_profiles.jl (7), runtests_kinetic.jl (5143) and runtests_slayer_runner.jl (164) pass.

On the Mac (Julia 1.11.6) at c05b2babc:

  • runtests_solve_api.jl passes 148/148, including the 4 new checks.
  • The Equilibrium - BUGFIX - Build the two-pass ψ grid from kinetic profiles that have no omega_E #614 reproducer, examples/DIIID-like_SLAYER_example with its kinetic file copied without omega_E, now runs to completion with the [SLAYER.CriticalResonantField] block disabled.
    • It logs "the kinetic profiles lack an NTV field …, so the grid follows the equilibrium alone".
    • SLAYER warns and takes Ω_E = 0.
    • With that block enabled, the run stops at the critical resonant field instead, with its own explicit error: it genuinely needs the E×B rotation.

Notes for reviewers

Cause. The two-pass grid (maybe_reform_equilibrium) and the EL kinetic stage (prepare_force_free_states!) built the NTV view of the attached profiles (ntv_profiles, ntv_species) whenever any profiles were attached. That view requires n_e, T_i, T_e and omega_E. A file read for SLAYER need not carry omega_E, and the HDF5 kinetic schema treats it as optional.

Change.

  • Equilibrium.has_ntv_fields(data) names the NTV view's required fields once.
  • _ntv_views(kf_ctrl, equil, ctrl) is the one place both call sites now get the view and species. It builds them when a calculated kinetic source needs them, where a missing field still errors from kinetic_profile_splines. Otherwise it builds them only when the profiles carry the NTV fields.
  • When the profiles lack those fields, the grid follows the equilibrium alone and logs that once. For a SLAYER-only deck that is the grid it had before kinetic profiles moved onto the equilibrium.
  • kinetic_forces_control stops a run with [KineticForces] or kinetic_source = "calculated" up front when the profiles lack an NTV field, before any solve.
  • The KineticForces stage, the Validity output and the error-field couplings still build the NTV view unconditionally. They are NTV consumers, so a missing omega_E should, and does, stop them.
  • When the grid has no NTV view and axis_validity_suppression is on, the EL stage logs that the near-axis suppression is off.

Unchanged. When the attached profiles carry every NTV field, which covers every shipped deck, both call sites behave exactly as before. That includes a fixed kinetic source with attached profiles, which keeps its axis-validity boundary.

Tests (extending "attached kinetic profiles refine the two-pass grid"):

  • has_ntv_fields separates the two profile sets.
  • omega_E-less profiles re-form without error, stay attached, and give exactly the grid of an equilibrium with no profiles attached.
  • A calculated kinetic source with the same profiles still fails, with "missing required 'omega_E'".
  • kinetic_forces_control stops on those profiles with [KineticForces] or a calculated source, and returns normally for a deck with neither.

🤖 Generated with Claude Code

…s that have no omega_E

The grid refinement and the EL kinetic stage built the NTV view of the attached profiles whenever any were attached, and that view requires omega_E, so a deck whose kinetic file has no omega_E (read for SLAYER) stopped in the equilibrium stage. The view is now built only when a calculated kinetic source needs it, where a missing field still errors, or when the profiles carry the NTV fields; otherwise the grid follows the equilibrium alone, as before the profiles moved onto the equilibrium.

Closes #614

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@d-burg d-burg added this to the GPEC v2.0.0 milestone Oct 30, 2026
@d-burg d-burg self-assigned this Oct 30, 2026
@github-actions github-actions Bot added the bugfix Something was wrong and now is not label Oct 30, 2026
@d-burg
d-burg marked this pull request as ready for review October 30, 2026 08:06

@jhalpern30 jhalpern30 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix looks right for the SLAYER-only case; the grid behavior matches the description. One accuracy concern (fail-late for [KineticForces] decks) and a few clarity nits inline.

Also: the maybe_reform_equilibrium docstring still says the grid measures "the NTV view of its attached kinetic profiles"; that is now conditional ("when they carry the NTV fields"). And the branch is behind develop.

Comment thread src/GeneralizedPerturbedEquilibrium.jl Outdated
# kinetic source needs them, so a missing profile errors; otherwise they are built only when the profiles
# carry the NTV fields, and profiles read for another consumer (e.g. SLAYER without omega_E) are left alone.
function _ntv_views(kf_ctrl::KineticForces.KineticForcesControl, equil::Equilibrium.PlasmaEquilibrium, ctrl::ForceFreeStatesControl)
needed = ctrl.kinetic_factor > 0 && ctrl.kinetic_source == "calculated"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fail-late regression for [KineticForces] decks. needed covers only the calculated source. A deck with a [KineticForces] section and profiles lacking omega_E used to error here in the equilibrium stage; now it runs the full FFS solve and writes HDF5 before erroring in write_validity! / the KineticForces stage.

Suggest adding the check to kinetic_forces_control, next to the existing "needs kinetic profiles" error: error when _needs_kinetic_profiles(inputs, ctrl) && !Equilibrium.has_ntv_fields(equil.kinetic). That keeps every NTV consumer failing up front. (needed here would then be redundant on the run_gpec path, but the solve-API path skips kinetic_forces_control, so keep it unless that path gets the same check.)

Minor: this is now the third copy of kinetic_factor > 0 && kinetic_source == "calculated" (with _needs_kinetic_profiles and resonance_grid_nodes).

Comment thread src/GeneralizedPerturbedEquilibrium.jl Outdated
kinetic_profiles = KineticForces.ntv_profiles(kf_ctrl, equil)
kinetic_profiles, species = _ntv_views(kf_ctrl, equil, ctrl)
(equil.kinetic === nothing || kinetic_profiles !== nothing) ||
@info "Two-pass psi grid: the kinetic profiles lack an NTV field (n_e, T_i, T_e, omega_E), so the grid follows the equilibrium alone"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the (a || b) || @info guard is a double negative; equil.kinetic !== nothing && kinetic_profiles === nothing && @info ... reads directly.

Comment thread src/GeneralizedPerturbedEquilibrium.jl Outdated
# importing KineticForces (which would invert the load order).
kinetic_profiles = KineticForces.ntv_profiles(kf_ctrl, equil)
species = KineticForces.ntv_species(kf_ctrl, equil)
kinetic_profiles, species = _ntv_views(kf_ctrl, equil, ctrl)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With kinetic_source = "fixed" and profiles lacking omega_E, this now gives axis_psi_c = 0, so near-axis suppression is silently off, while the same deck with omega_E gets a nonzero boundary. Not a regression (it errored before), but worth an @info here or a line in the PR description so it isn't silent.

Comment thread test/runtests_solve_api.jl Outdated
# equilibrium alone, unless the run computes calculated kinetic matrices, which need it.
no_omega = GPEC.Equilibrium.KineticProfileData(; psi=kinetic.psi, n_i=kinetic.n_i, n_e=kinetic.n_e,
T_i=kinetic.T_i, T_e=kinetic.T_e, chi_e=kinetic.chi_e, chi_i=kinetic.chi_i, chi_phi=kinetic.chi_phi)
@test !GPEC.Equilibrium.has_ntv_fields(no_omega) && GPEC.Equilibrium.has_ntv_fields(kinetic)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: split into two @tests so a failure says which half broke.

@jhalpern30 jhalpern30 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, up to you what you want to include from the review

…ga_E and report the suppression they lose

A [KineticForces] deck with omega_E-less profiles now errors in kinetic_forces_control rather than after the solve, a fixed kinetic source logs that the near-axis suppression is off, and the calculated-source test lives in one helper. Review changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…id-without-omega-e

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@logan-nc
logan-nc added this pull request to the merge queue Nov 1, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Nov 1, 2026
…NTV-view guard to the force-balance solve

The solve moved into ForceFreeStates with #602: the grid guard now lives in KineticForces.kinetic_grid_inputs, the up-front checks in _prepare_solve and main_from_inputs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186CLfjeJeSusNTva5RMfGU
Copilot AI balanced review requested due to automatic review settings November 1, 2026 23:27
@logan-nc

logan-nc commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator

@d-burg @jhalpern30 @matt-pharr Heads up: we've taken this over and will carry it through to the merge, because #602 landed first and this PR now conflicts with develop.

Pushed (4bba6e7a5): develop merged in, with the fix ported to where #602 moved the solve:

  • KineticForces.kinetic_grid_inputs builds the NTV view only for the KineticForces source, or when the profiles carry n_e, T_i, T_e and omega_E. Otherwise the two-pass grid follows the equilibrium alone and logs that.
  • _prepare_solve stops a KineticForces solve before any work when an NTV field is missing.
  • main_from_inputs stops a [KineticForces] deck right after the equilibrium stage, not after the perturbed equilibrium (Jake's fail-late case).
  • The near-axis-suppression notice is dropped: on develop, only the KineticForces source computes that boundary, and that source already requires the profiles.
  • The test is updated to the new API.

Running now: runtests_solve_api, runtests_grid_refinement, runtests_kinetic_profiles and runtests_slayer_runner, plus the full harness (develop e51f4b0ee against 4bba6e7a5). I'll update the stamp and report when they finish, then put it back in the merge queue.

@jhalpern30, your approval was for the pre-port code. Please take a quick look at the port (+39/−3 against develop) when you can. Please don't push to this branch in the meantime, to avoid collisions.

@logan-nc

logan-nc commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator

Tests pass at 4bba6e7a5: runtests_solve_api 171/171, runtests_grid_refinement 83/83, runtests_kinetic_profiles 7/7 and runtests_slayer_runner 151/151. The full harness is still running; the stamp follows when it finishes.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open issues
What changed in this PR

Fixes two-pass ψ grid construction when kinetic profiles lack omega_E, allowing SLAYER-only runs to proceed by falling back to an equilibrium-only grid while still failing fast for stages that truly require the NTV view.

Changes:

  • Add Equilibrium.has_ntv_fields to centralize the required-field check for the NTV view.
  • Update two-pass grid inputs to skip building NTV views when profiles are missing required fields (unless the kinetic source requires them).
  • Add early validation to stop runs up-front when [KineticForces] / calculated kinetic sources require omega_E, plus new regression tests.
File Description
test/​runtests_solve_api.jl Adds regression coverage for ωE-less profiles behavior and early-stop behavior.
src/​KineticForces/​CalculatedKineticMatrices.jl Skips NTV-view construction for ωE-less profiles unless required; logs fallback.
src/​GeneralizedPerturbedEquilibrium.jl Adds early validation error for [KineticForces] when NTV fields are missing.
src/​ForceFreeStates/​ForceBalance.jl Adds validation that KineticForces kinetic source has required NTV fields.
src/​Equilibrium/​KineticProfiles.jl Introduces has_ntv_fields helper and docs.

Review effort: Lite

Comment on lines +179 to +181
@test_throws "missing required 'omega_E'" GPEC.maybe_reform_equilibrium(pass1, sl_intr, calc_ctrl)
# The KineticForces source stops up front, before any solve.
@test_throws "omega_E" solve(pass1, FFS.Forward(); nn=1, kinetic=GPEC.KineticForcesControl())

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not an issue: since Julia 1.8, @test_throws accepts a string and matches it against the error message. These assertions pass (runtests_solve_api 171/171 at 4bba6e7a5), and a wrong message would make them fail.

Comment on lines +170 to +171
deck.kinetic_forces !== nothing && equil.kinetic !== nothing && !Equilibrium.has_ntv_fields(equil.kinetic) &&
error("[KineticForces] needs n_e, T_i, T_e and omega_E in the kinetic profiles; the kinetic file lacks at least one")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair, but leaving it for now. The message lists the four required fields, and if the run got further, kinetic_profile_splines would name the missing dataset itself. Changing the wording now would touch src/ and invalidate the harness stamp that is running for the v2.0 deadline. Happy to take it as a follow-up.

data = equil.kinetic
ntv = source isa KineticForcesControl || (data !== nothing && Equilibrium.has_ntv_fields(data))
data !== nothing && !ntv &&
@info "Two-pass psi grid: the kinetic profiles lack an NTV field (n_e, T_i, T_e, omega_E), so the grid follows the equilibrium alone"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kinetic_grid_inputs is called once per solve, from maybe_reform_equilibrium, so this logs once per run. No change needed.

@logan-nc
logan-nc added this pull request to the merge queue Nov 1, 2026
Merged via the queue into develop with commit fb93b15 Nov 2, 2026
21 of 23 checks passed
@logan-nc
logan-nc deleted the bugfix/two-pass-grid-without-omega-e branch November 2, 2026 00:15
@logan-nc

logan-nc commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

Harness record, run after merge: regress --refs e51f4b0ee,4bba6e7a5 (all 25 cases) on alfven, 16 threads, Julia 1.11.7, manifest pinned. Every tracked quantity is unchanged. gal_resistive_pe reports N/A on both refs, develop included, so it is a stale case definition and not this PR.

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

Labels

bugfix Something was wrong and now is not

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Equilibrium - BUGFIX - Build the two-pass ψ grid from kinetic profiles that have no omega_E

4 participants