Skip to content

fix(core): correct Crucible-confirmed QS + spectral physics defects#60

Closed
jungdaesuh wants to merge 1 commit into
developfrom
bugfix/qs-spectral-fit-math
Closed

fix(core): correct Crucible-confirmed QS + spectral physics defects#60
jungdaesuh wants to merge 1 commit into
developfrom
bugfix/qs-spectral-fit-math

Conversation

@jungdaesuh

Copy link
Copy Markdown
Collaborator

Summary

Fixes five confirmed correctness/contract defects in the QS + spectral core, surfaced
by an adversarial (Crucible) review. Each is covered by a regression test that fails
against the pre-fix behavior. No behavioral scope creep.

Related issue

Relates to #40 — three of these are in the pure core/quasistationary port, a
prerequisite to wiring it in place of the _slcontour reference.

Type of change

  • Analysis core (src/magnetics/core)
  • Data source (src/magnetics/data)
  • Service / API (src/magnetics/service)
  • Web GUI (gui/web)
  • Docs
  • Other

What changed

  • qs_bridge: fit-quality status vocabulary (ok/warn/error) diverged from the
    contract's good/warn/bad, so the GUI traffic light lost its color for healthy /
    ill-conditioned fits → route through contracts.quality_for_k (SSOT).
  • _slcontour/fit.py + core/quasistationary.py: per-coefficient SVD σ summed the
    wrong axis (axis=01/w_k² by singular position, not the covariance diagonal) →
    wrong live QS error bars; axis=1 also sizes fit_sigmas to n_cols, fixing an
    underdetermined-fit IndexError.
  • _slcontour/fit.py: the 2D sinusoidal-integral basis divided by
    deg2rad(dx*dy)*n*m (π/180 once, no i²) → m≠0 columns off by −180/π; use the
    separable product of the two 1D branches.
  • core/quasistationary.py reconstruct_grid: used c instead of conj(c) vs its
    own forward model → toroidal mirror.
  • core/spectral.py: phase_sigma reported the inverse-variance (BLUE) variance
    while the intercept is amplitude/coherence-weighted → propagate σ through the same
    weights (honest, non-optimistic 1σ).

Note: the review's initially-reported except-syntax "critical" was a false
positive
(valid PEP 758 on the pinned Python 3.14). No change here.

How I tested it

uv sync --extra service; uv run ruff format --check . / ruff check . /
ty check src/magnetics → clean; uv run --with pytest pytest -q → green
(tests/test_crucible_regressions.py + a test_qs_bridge quality-vocab test, each
failing against the pre-fix behavior).

Checklist

  • CI is green (analysis: ruff + pytest; web: lint + build) — validated locally
  • No secrets/credentials committed
  • No large files or tokamak data committed
  • Physics kept in core (device-agnostic); none added to service routes
  • Docs updated if behavior/usage changed (n/a)

🤖 Generated with Claude Code

- qs_bridge: fit-quality status used ok/warn/error, which the contract's
  Quality (good/warn/bad) and NodeView.QCOLOR can't color; route through
  contracts.quality_for_k (SSOT).
- _slcontour/fit.py + core/quasistationary.py: per-coefficient SVD σ summed the
  wrong axis (axis=0 → 1/w_k² by singular position, not the covariance diagonal);
  axis=1 fixes the QS error bars and sizes fit_sigmas to n_cols so underdetermined
  fits no longer IndexError.
- _slcontour/fit.py: 2D sinusoidal-integral basis divided by deg2rad(dx*dy)*n*m
  (π/180 once, no i²) → m≠0 columns off by -180/π; use the separable product.
- core/quasistationary.py reconstruct_grid: used c instead of conj(c) vs its own
  forward model (and qs_bridge's sign) → toroidal mirror; use conj(c).
- core/spectral.py: phase_sigma reported the inverse-variance (BLUE) variance
  while the intercept is amplitude/coherence-weighted; propagate σ through the
  same weights so the reported 1σ matches the returned estimator.

Adds tests/test_crucible_regressions.py + a qs_bridge quality-vocab test, each
failing against the pre-fix behavior. ruff + ty(src) clean; full suite green.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@matt-pharr matt-pharr requested a review from priyanshlunia July 1, 2026 19:18
@matt-pharr

Copy link
Copy Markdown
Collaborator

Superseded by #68, which is a rebased superset of this PR on current develop (contains all five fixes here: the SVD-σ axis=1 fix, the 2-D sinusoidal-integral denominator, the conj(c) reconstruction, the quality_for_k status vocab, and the weighted-σ phase_sigma). Closing as a duplicate — review/merge #68 instead.

@matt-pharr matt-pharr closed this Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants