Bugfix/bandpass fix#84
Open
mfairborn23 wants to merge 19 commits into
Open
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.
Pulls in the cluster-network-config work that landed on develop while this branch was in progress (device-file network blocks, explicit cluster address + per-channel mdsthin default, auto hop-count resolution) plus new QS-correctness test coverage (PR #47). No conflicts with this branch's QS GUI/backend work.
Mirrors qs_plots.plot_svds's two diagnostic panels as GUI-servable LineNodes: data-matrix SVD cumulative energy fraction and design-matrix per-singular-value condition number, each carrying the relevant threshold (energy_limit / fit_condition) as a reference line. Co-Authored-By: Claude Sonnet 5 <[email protected]>
…lots - Add hover title= tooltips (sourced from OMFIT's magnetics_gui.py help text) to Array, n/m modes, Detrend, band, t trim, and basis function - the last gaps among the QS settings-bar and Advanced Options controls. - Surface svd_energy/svd_condition fetch errors instead of silently leaving the panels stuck on a generic "loading..." placeholder forever. - Move the SVD energy/condition plots into their own default-open section instead of nesting them inside the closed-by-default "fit quality" panel, so they're visible without extra clicks. - Fix SVD condition-number axis rendering: use a linear y-axis to match the qs_plots.plot_svds reference (was hardcoded log, which let an off-screen threshold line stretch the autorange and squash the data), add dtick=1 to both SVD x-axes (small-integer singular-value index), and give both legends more top margin so they don't crowd the header. Co-Authored-By: Claude Sonnet 5 <[email protected]>
- SVD conditioning: default the section closed like the other collapsible panels, and double both plots' height (150 -> 300). - Reorder the settings bar: move "t trim (ms)" next to "Array". - Shorten the Array tooltip to plain dropdown guidance. - Rename "band (ms)" to "detrend band (ms)" for clarity. Co-Authored-By: Claude Sonnet 5 <[email protected]>
Main-series points 4 -> 8, "removed" x-markers 7 -> 12. Co-Authored-By: Claude Sonnet 5 <[email protected]>
# Conflicts: # gui/web/src/components/tabs/QuasiStationaryTab.tsx
…l when you click the button to adjust text size
Replace the primary-bar + Advanced-toggle split with three always-visible rows grouped by function: Basics (array, time trim, mode numbers), Data (detrend, bandpass, SVD filtering, uncertainty), and Fitting (basis function, condition number). Removes the advancedOpen toggle so nothing is hidden by default. Co-Authored-By: Claude Sonnet 5 <[email protected]>
Disabling the bandpass filter (cutoff_lo=0, cutoff_hi=inf) skipped the decimation step too, since both lived behind the same "do we need to filter" gate. A shot's full native sample count (10^5-10^6) then flowed through SVD/fit and got JSON-serialized whole across ~11 parallel node fetches per "Plot" click — over a gigabyte of response, crashing the tab with a stack overflow in Math.min(...)/Math.max(...) range calculations. Add an unconditional _MAX_PREP_SAMPLES=2000 cap after the time-trim, decoupled from whether the frequency filter itself runs. Matches the existing precedent in nodes.py's _raw_trace (also capped at 2000). Verified against shot 184927: phi_t drops from 521 MB to 1.2 MB with cutoff_lo=0/cutoff_hi=inf; full test suite and ty typecheck pass. Co-Authored-By: Claude Sonnet 5 <[email protected]>
…so that if you zoom in on a subset of the graph, you can specify which angles (what y-axis) we have with more precision than click and drag. This probably shouldn't be on this branch, but this was the branch I had open
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.
Bandpass-disable crash fix — summary
Branch:
bugfix/bandpass-fix(based onqs-gui-improvement), commit0e18a08.PR (base
qs-gui-improvement← comparebugfix/bandpass-fix):https://github.com/OpenFUSIONToolkit/Magnetics/pull/new/bugfix/bandpass-fix
Notes
Handles the same issues as PR #57 since it was branched off of
qs-gui-improvementbut also potentially fixes a bug so the user can choose not to use a bandpass filter (set the min and max to 0 and inf respectively. This is mentioned in Issue #83.The bug
The bandpass control has a documented convention for "no filtering" —
cutoff_lo=0,cutoff_hi=inf.qs_prep.pyalready treated that as a passthrough by skipping the filterblock. The problem was that the same
ifalso gated decimation — so skipping thefilter also skipped the downsampling that normally keeps the output small. A shot's full
native sample count (100k–1M+ samples) then flowed straight through SVD/fit and got
serialized to JSON whole. The QS tab fires ~11 node fetches in parallel per "Plot" click,
so one click could ask for over a gigabyte of JSON — which is what actually crashed the tab
(
Maximum call stack size exceeded, thrown fromMath.min(...)/Math.max(...)spread callsin
QuasiStationaryTab.tsxoperating on the oversized arrays).The fix
Added an unconditional
_MAX_PREP_SAMPLES = 2000cap insrc/magnetics/core/qs_prep.py,applied right after the time-trim, independent of whether the frequency filter runs. This
matches an existing precedent elsewhere in the codebase (
nodes.py's_raw_trace, alsocapped at 2000). Verified against real shot 184927:
phi_twent from 521 MB → 1.2 MB,signal_conditioning420 MB → 0.77 MB, with the exactcutoff_lo=0/cutoff_hi=infparamsthat triggered the crash. Full test suite (327 passing) and
tytypecheck are clean.Branch/target note
The branch/PR targets
qs-gui-improvement, notdevelop— the realorigin/developdoesn't have
core/qs_prep.pyyet (it's only on feature branches so far), so a PR againstliteral
developwould've shown the whole file being created instead of just this fix.Once the QS pipeline lands on
develop, this fix should follow the same path.PR title/body
Title:
Fix crash when bandpass filtering is disabled (0 → inf)Summary
Fixes a crash reported as: "The qs view failed to render. Maximum call stack size exceeded."
The QS tab's bandpass control supports a documented convention for disabling filtering
entirely — cutoff_lo=0, cutoff_hi=inf (or a very large number) — which qs_prep.py already
treated as a full passthrough by skipping the filter block.
The bug: that same
ifgate also controlled decimation, not just the filter itself. With(0, inf), decimation was skipped too, so a shot's full native sample count (10^5-10^6
samples) flowed unbounded through SVD/fit and got JSON-serialized whole. The QS tab fires
~11 node fetches in parallel on "Plot", so one click could request over a gigabyte of
JSON — the browser tab's stack overflow (in Math.min(...)/Math.max(...) range calculations
in QuasiStationaryTab.tsx) is a direct symptom of that.
Measured against real shot 184927 before the fix:
Fix
src/magnetics/core/qs_prep.py: add an unconditional _MAX_PREP_SAMPLES = 2000 cap, applied
after the time-trim and decoupled from whether the frequency filter itself runs. Matches
the existing precedent in service/nodes.py's _raw_trace (also capped at 2000 samples,
"keep the line light"). The cap protects the (0, inf) passthrough case and any other
permissive-but-finite cutoff that happens to leave decimation at a no-op.
After the fix, the same repro: phi_t → 1.2 MB, signal_conditioning → 0.77 MB — back in
line with normal-case magnitude.
Test plan
420 MB payloads)
nodes.build_node() directly and via the live /api/node/... endpoint
Branch note
This targets qs-gui-improvement rather than develop — core/qs_prep.py isn't merged into
develop yet, only onto feature branches. Once the QS pipeline lands on develop, this fix
should follow the same path.