Pairwise-difference basis for differential magnetic sensors#80
Open
priyanshlunia wants to merge 12 commits into
Open
Pairwise-difference basis for differential magnetic sensors#80priyanshlunia wants to merge 12 commits into
priyanshlunia wants to merge 12 commits into
Conversation
Retire the dead core/quasistationary.py (a partial, unwired port whose
reconstruct_grid used exp(+i(nφ+mθ)) — wrong for any phased mode) plus its
falsely-certifying test, and promote the *active* SLCONTOUR pipeline into core
under unified qs_ naming, deleting the omfit_compat OMFIT shim.
- Move _slcontour/{fit,prep,io_data,run,plots}.py -> core/qs_*.py.
- Delete omfit_compat.py, folding each symbol to its owner: printv/w/e -> logging;
OMFITexception -> ValueError; delta_degrees -> qs_fit; cornernote/uband ->
qs_plots; device readers -> new core/qs_device.py, which delegates to the data
layer (data.devices / data.diiid_geometry) and keeps only the QS-specific extras
(derived theta/end-coord sensor_geometry, resolve_channel_filter semantics).
- qs_device.load_wall now delegates to devices.feature_at, fixing the segmented-
schema bug (the shim read a nonexistent top-level `wall` key -> (None, None)).
- Lock the -i reconstruction convention: invariant comments at the three sites
+ a discriminating δ≠0 regression test the old zero-phase test missed.
- qs_plots docstring clarified as standalone matplotlib (not GUI/service wired).
- Rewire service/nodes.py + tests; pyproject drops the ruff exclude and retargets
a deferred ty exclude onto the qs_* files. Relocate the reference notebook/README
to docs/qs_slcontour/.
ruff format/check + ty (with service extra) clean; 209 passed, 1 skipped.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Finish the core/qs_* promotion by cleaning up its reference material, which was
still written for the old OMFIT `magnetics-code` layout.
- Move example_magnetics.ipynb into examples/ and rewire it onto the new
infrastructure: imports become magnetics.core.qs_{io_data,device,run} plus
`qs_plots as plots` (so the plot cells are unchanged); drop the sys.path hack
and the OMFIT-script mapping / "ported OMFIT scripts" framing.
- Replace docs/qs_slcontour/README.md with docs/quasistationary-mode-analysis.md:
no OMFIT-magnetics plumbing (module↔OMFIT-script table, omfit_compat, cd
analysis), instead a walkthrough of the qs_* pipeline (io_data → prep → fit →
run, plus qs_bridge→service and standalone qs_plots), data inputs, quick start,
and the -i reconstruction sign convention. Physics provenance (SLCONTOUR,
VISION §4.1) kept.
- Remove the now-empty docs/qs_slcontour/.
Docs-only; no src/ behavior change. ruff format --check clean.
(docs/magnetics-code-architecture.md is also OMFIT-era — left for a later sweep.)
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…ond), wall-outline fix - QS tab's Array dropdown now uses the same device sensor_sets list as the left sidebar instead of a hardcoded 9-item subset. - New Advanced Options controls in the QS tab: uncertainty (sigma), fraction of energy included, basis function, fit condition - wired through to the SLCONTOUR fit via new sigma_override/fit_basis/fit_cond query params. - Fix load_wall() reading the device JSON's legacy flat wall schema; it's been shot-segmented since 28c75c6, so the wall outline silently stopped rendering in the QS Sensor Map cross-section plot. Co-Authored-By: Claude Sonnet 5 <[email protected]>
Prioritizes the incoming SLCONTOUR restructuring (_slcontour/* -> core/qs_*, qs_device.py replacing omfit_compat.py) over this branch's prior fixes where they overlapped: - Dropped this branch's _segment_fields-based load_wall fix on the legacy `wall` key in favor of the incoming qs_device.load_wall, which resolves the newer, canonical `first_wall` key via devices.feature_at. - Kept this branch's additive work with no incoming equivalent: the QS tab's Advanced Options UI, and the sigma_override/fit_basis/fit_cond query-param wiring in _qs_run/_prep_qs_ds (now pointed at core.qs_run/core.qs_device instead of the deleted _slcontour module). - Merged both branches' new tests in test_qs_bridge.py and test_slcontour_geometry.py rather than dropping either side's coverage.
The QS fit hardcoded `dp = None` and never differenced the basis for
differential magnetic sensors (the MPID/ISLD/ESLD families), so a sensor that
physically reports field(X) - field(pair) was fit against a single-location
basis. This affected the default production fit: all 10 MPID66M* probes of
Bp_LFS_midplane are paired. Restores the OMFIT fit_magnetics.py treatment
(design-matrix column = (basis(X)/sigma - basis(pair)/sigma_pair) / 2), applied
per-channel so mixed paired/unpaired arrays are handled.
- qs_device.sensor_geometry: resolve each channel's pair in the full device
table and emit pair_{phi,theta,z}_end1/2 vars (NaN when unpaired), riding the
existing geometry-attach path to the fit.
- qs_fit.fit: difference the basis where has_pair (main loop + ill-conditioned
refit, with the x0/y0 shift on the pair too); psigma = sigma under the constant
per-shot sigma; fix the n=0 warning to fire only when unpaired; emit an
n_paired attr. Gaussian path gets the same treatment for parity.
- synthetic_shot: make paired sensors differential (signal(X) = field(X) -
field(pair)) so the fixture stays physically consistent with the fit.
- tests/test_qs_pairdiff.py: unit test that the paired design column equals
(basis(X) - basis(pair)) / 2 and the unpaired one equals basis(X); pipeline
test that Bp_LFS_midplane reports n_paired == 10.
Per-channel detection is identical to OMFIT's all-or-nothing on every real
(uniformly paired/unpaired) DIII-D array. ruff/ty clean; 218 passed, 1 skipped.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Brings develop through #46 (gui-sensors): NSTX/NSTX-U device-generic fetch, per-node plot + HDF5 export, EFIT tree-fetch, and the data-layer device_geom refactor. No merge conflicts; the qs_* pipeline works against develop's refactored data layer. Verified on the merged tree: ruff format/check clean, ty clean, 290 passed / 5 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…core Two device-specific bits removed from the QS core (issue #40 device-agnostic goal), without touching data/device/*.json or the data layer: - qs_device: delete the hardcoded `_DEVICE_ALIAS = {"DIII-D": "diiid"}` and `_device_key`; `load_device` now delegates to `devices.resolve_device_id`, which derives the config id from each device JSON's `name` field. - qs_fit: remove the advisory `is_device("DIII-D")` + `C.*/IL.*/IU.*` coil-basis warning and its now-unused `is_device` import. The 2019 ESLD wiring swap in qs_prep is intentionally kept for now (migrating it needs a JSON `corrections` block + reader, deferred). ruff/ty clean; 290 passed. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Out of this branch's original (pairwise-difference) scope — an opportunistic fidelity fix spotted while investigating the example notebook. qs_io_data hardcoded the plasma `helicity` attr to -1, even though it loads the shot's `ip`/`bt` traces. helicity sets the fitted poloidal-mode (m) sign convention, so a blanket -1 is silently wrong for reversed-Bt/Ip shots. Now `_compute_helicity` derives it as sign(median Bt)/sign(median Ip) (matching OMFIT init_magnetics); the `helicity=-1` argument is kept only as a fallback for shots missing those traces. - core/qs_io_data.py: add `_compute_helicity`, wire into `_build_plasma`, clarify the fallback-only role of the parameter. - tests: unit coverage of the sign logic + a pipeline check that the synthetic shot's helicity is the computed +1, not the fallback. - docs (quasistationary-mode-analysis.md, CLAUDE.md): helicity is no longer an open item; the remaining QS-fidelity gap is per-sensor sigma. No effect on the default ms=(0,) fits (helicity only flips non-zero m). ruff/ty clean; 301 passed, 5 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
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
2 participants
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.
Pairwise-difference basis for differential magnetic sensors (+ helicity from Ip·Bt)
Summary
This PR gives the quasi-stationary fit correct handling of paired (differential) magnetic sensors —
something the previous iteration of the port silently did not do. The fit now restores the OMFIT
magnetics pairwise-difference treatment, and goes one step further: it supports mixed paired /
unpaired sensor arrays, which the original OMFIT treatment could not. One smaller improvement rides
along: helicity is now computed from the shot data instead of hardcoded.
Scope
This branch also merges in the advanced-GUI QS-tab work and the underlying shim-free
core/qs_*refactor. Those are covered by their own PRs and are not the focus here — this description
intentionally does not highlight them. The substantive new work in this PR is the pairwise-difference
treatment and the helicity fix below.
1. Paired sensors — the headline
The previous code ignored them
Many DIII-D magnetic sensors are differential: the
MPID/ISLD/ESLDfamilies (the "D" =difference) physically report
field(X) − field(pair), not the absolute field at one location. TheOMFIT magnetics
fit_magnetics.pyhandles this by differencing the basis to match the measurement.The previous iteration of our port dropped that entirely — it hardcoded
dp = Noneand fit everysensor against a single-location basis. This is not a corner case:
MPID66M*probes of
Bp_LFS_midplaneare paired.amplitudes/phases, inflates χ², and distorts the condition number K.
Now implemented with the OMFIT treatment
The design-matrix column for a paired channel is now the OMFIT formula
(
fit_magnetics.py:340— "subtract the same thing for the paired sensor if there is one"):applied in both the main design-matrix loop and the ill-conditioned-refit branch (with the same
x0/y0shift on the pair), plus the Gaussian path for parity. The signal side is left raw, exactly asOMFIT does. The inverted
n=0warning is corrected (it now fires only for unpaired arrays), and thefit reports an
n_pairedattribute.The paired sensor's geometry is resolved from the full device table in
qs_device.sensor_geometry(emitted aspair_{phi,theta,z}_end1/2vars) and rides the existinggeometry-attach path into the fit — so
qs_fitstays device-agnostic and a pair need not be among thefetched/fit channels.
Beyond OMFIT: mixed paired + unpaired arrays
OMFIT's detection is all-or-nothing: if any channel in the array is unpaired, it differences
nothing. This implementation is per-channel — it differences each channel that has a resolvable
pair and leaves unpaired channels single-location. So a mixed array is handled correctly, which was
not possible under the original OMFIT treatment (which would silently drop all pairing).
On any uniformly paired-or-unpaired array — i.e. every real DIII-D array (
MPID/ISLD/ESLDfullypaired;
MPI/coils fully unpaired) — the two are identical, and the per-sensor math is byte-for-bytethe OMFIT formula. The generalization only changes behavior on genuinely mixed arrays.
Tests
tests/test_qs_pairdiff.py): asserts a paired channel's design column equals(basis(X) − basis(pair))/2and an unpaired one equalsbasis(X)— on a mixed 2-channel array.Bp_LFS_midplanereportsn_paired == 10.physically consistent with the corrected fit.
2. Helicity computed from Ip·Bt (fidelity fix)
qs_io_datahardcoded the plasmahelicityattr to-1, even though it loads the shot'sip/bttraces. Helicity sets the fitted poloidal-mode (
m) sign convention, so a blanket-1is silentlywrong for reversed-Bt/Ip shots. It's now derived as
sign(median Bt) / sign(median Ip)(matching OMFITinit_magnetics); the-1parameter is kept only as a fallback for shots missing those traces. Noeffect on the default
ms=(0,)fits (helicity only flips non-zerom).Verification
uv run ruff format --check .+uv run ruff check .— clean.uv run --extra service ty check src/magnetics— clean.uv run --extra service pytest— 301 passed, 5 skipped.Reviewer notes
divergence is the per-channel generalization for mixed arrays.
qs_prep(kept intentionally; its JSONmigration is a separate follow-up).
core/qs_*refactor commits (their own PRs) — reviewthose there; focus review here on
qs_fit/qs_device/qs_io_dataand their tests.🤖 Generated with Claude Code