Skip to content

Correct no-GS coupling norms and preserve timescale selection - #94

Merged
kpentland merged 7 commits into
mainfrom
fix-no-gs-mode-coupling-norm
Aug 19, 2026
Merged

Correct no-GS coupling norms and preserve timescale selection#94
kpentland merged 7 commits into
mainfrom
fix-no-gs-mode-coupling-norm

Conversation

@nicamo

@nicamo nicamo commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • correct the no-GS plasma-coupling norm so each response is normalised by the perturbation that produced it;
  • add an explicit mode_selection policy separating coupling-based reduction from timescale-only reduction;
  • make the MAST-U growth-rate regression explicitly use and verify timescale-only selection;
  • document the equilibrium-local nature of coupling-ranked selection in the API and examples;
  • merge current main in c38b910.

Merge order

This PR should be merged after #93. That PR corrects the normal-mode Green maps used when evaluating the no-GS plasma responses; this PR then corrects how those responses are normalised and used for mode selection. After #93 lands, this branch must be updated and the mode-selection and dynamics regressions rerun. The shared growth-rate notebook and test should retain both #93's Green-map correction and this PR's explicit timescale-selection policy.

Coupling-norm correction

prepare_build_dIydI_j() returns a finite-difference column

dIydInoGS = delta_Iy / delta_I_used.

It also prepares delta_I_next for the later full Jacobian. Previously, build_dIydI_noGS() divided the already normalised column by delta_I_next, mixing two finite-difference evaluations. The stored coupling metric is now simply:

self.ndIydI_no_GS[j] = np.linalg.norm(dIydInoGS)

This correction does not change perturbation calibration or the subsequent full-GS Jacobian calculation.

Mode-selection policies

The new keyword argument is appended to the constructor signature, preserving positional compatibility. Its default is mode_selection="coupling", which preserves existing behavior.

mode_selection="coupling" (default)

The no-GS calculation both calibrates finite-difference perturbations and estimates plasma coupling.

  • With fix_n_vessel_modes=N, retain the N strongest-coupled passive modes.
  • With fix_n_vessel_modes=-1, combine max_mode_frequency with the no-GS coupling thresholds.
  • If requested, mode_removal may subsequently remove weakly coupled modes using the full-GS Jacobian.

The norm correction in this PR affects this policy, as intended.

Scope and caution: the coupling ranking is calculated about the equilibrium supplied at initialisation. Passive-mode coupling can evolve with plasma position and shape, so an initially selected reduced basis may cease to be representative during a substantially evolving trajectory. For such cases, the retained set should be validated across representative equilibria, or mode_selection="timescale" should be used for equilibrium-independent selection. This caveat is now stated in the solver API, lower-level mask documentation, growth-rate example, and pulse-design example.

mode_selection="timescale"

The no-GS calculation is still used to calibrate finite-difference perturbation sizes, but its coupling norms do not affect which modes are retained.

  • With fix_n_vessel_modes=N, retain exactly the N lowest-frequency (longest-timescale) passive modes.
  • With fix_n_vessel_modes=-1, retain only modes below max_mode_frequency.
  • Coupling-based post-Jacobian mode removal is disabled, so the policy remains genuinely timescale-only.

The lower-level selector rejects attempts to combine a fixed timescale count with coupling masks.

Dynamics regression

The review discussion exposed that the growth-rate fixture's intended policy was ambiguous. Under the previous implicit coupling policy, correcting the norm swapped seven of the 50 selected passive modes and changed the instability timescale from 0.05857045 s to 0.05900355 s.

The fixture now requests mode_selection="timescale", asserts that passive modes 0..49 are retained, and obtains 0.05899346 s (reference 0.05900 s at the existing 0.1% tolerance). This reference is independent of the corrected coupling ranking.

Validation

  • focused mode-selection tests: 3 passed;
  • current full suite: 39 passed, 4 skipped;
  • black --check and isort --check-only pass for all modified files;
  • both modified notebooks remain valid JSON;
  • GitHub CI: 4/4 checks passed, including the full notebook workflow (virtual circuits, growth rates, and pulse design).

@nicamo nicamo added the bug Something isn't working label Aug 4, 2026
@nicamo
nicamo changed the base branch from streamlining to main August 4, 2026 11:12
@kpentland kpentland self-assigned this Aug 11, 2026
Comment thread freegsnke/tests/test_dynamics.py
@nicamo nicamo changed the title Correct no-GS mode coupling norms Correct no-GS coupling norms and preserve timescale selection Aug 11, 2026
@nicamo nicamo added the ready-for-final-tests Pull request is ready to run final pre-merge tests label Aug 11, 2026
@nicamo nicamo added ready-for-final-tests Pull request is ready to run final pre-merge tests and removed ready-for-final-tests Pull request is ready to run final pre-merge tests labels Aug 11, 2026
@kpentland kpentland removed the ready-for-final-tests Pull request is ready to run final pre-merge tests label Aug 19, 2026

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.

Nice clear updates.

I've also added a few extra details to the Portone and Leuer stability parameters to help users interpret their meaning (not sure why I didn't add this before...)

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.

Changes make the mode selection much clearer - how to do this is now reflected in Example10.

@kpentland kpentland added the ready-for-final-tests Pull request is ready to run final pre-merge tests label Aug 19, 2026
@kpentland
kpentland merged commit dc3c927 into main Aug 19, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready-for-final-tests Pull request is ready to run final pre-merge tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants