From 8a01d4d3412804a77f9eca0ff2ebe2abcc92229f Mon Sep 17 00:00:00 2001 From: Matthew Pharr Date: Mon, 27 Jul 2026 03:31:31 -0400 Subject: [PATCH] feat: exclude bad sensors from all analysis (#56) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A misbehaving probe — dead channel, railed signal, wrong gain — flowed straight into both analyses and silently skewed the fits, with no way to drop it short of editing code. Click a sensor in the Sensors view to exclude it. Exclusions are saved per shot (that is how bad channels behave: bad *on a shot*), so they survive a reload and are shared by anyone opening that shot. The seam sits below both analyses rather than per view: * rotating — _array_channels() drops them, so every toroidal/poloidal node inherits it via _toroidal_arr/_pick_pair/_toroidal_grid; a new call site cannot forget to honor it; * quasi-stationary — unioned into the existing fit_exclude, so the QS tab's own per-fit deselect and the shot-wide bad list both reach the fit. Cache correctness is the subtle half. Most analysis caches key on an explicit channel-name list, which already moves when the channel set changes — but _spec_result keys on (shot, stft params) while calling _pick_pair internally, so excluding a probe could change the pair and still serve the cached pre-exclusion STFT. It now takes the exclusion tuple as a key argument (test pins this). Client-side, useNode folds a store revision counter into its fetch key, so one toggle re-fits every open view without threading it through ~30 calls. Excluded sensors stay on the map, greyed and clickable, so you can see what you dropped and restore it — plus a banner listing them with restore/clear. Verified live with Playwright against real shot 184927: click → persisted → honored by the geometry node → survives reload → restore clears it. Closes #56 Co-Authored-By: Claude Opus 5 --- .gitignore | 4 + gui/web/src/components/tabs/SensorsTab.tsx | 156 ++++++++++++++++--- gui/web/src/lib/api.ts | 20 +++ gui/web/src/lib/exclusions.test.ts | 98 ++++++++++++ gui/web/src/lib/useNode.ts | 8 +- gui/web/src/store.ts | 47 ++++++ src/magnetics/data/exclusions.py | 108 +++++++++++++ src/magnetics/service/app.py | 23 ++- src/magnetics/service/nodes.py | 64 ++++++-- tests/test_exclusions.py | 172 +++++++++++++++++++++ 10 files changed, 665 insertions(+), 35 deletions(-) create mode 100644 gui/web/src/lib/exclusions.test.ts create mode 100644 src/magnetics/data/exclusions.py create mode 100644 tests/test_exclusions.py diff --git a/.gitignore b/.gitignore index 5fc553e..494c821 100644 --- a/.gitignore +++ b/.gitignore @@ -32,6 +32,10 @@ src/magnetics/service/webapp/ data/cache/ *.nc +# Per-shot bad-channel exclusions (#56): local operator state written next to the +# shot data at runtime, not repo content. +data/exclusions.json + # HDF5 shot data — large, fetched/generated locally, not committed *.h5 *.hdf5 diff --git a/gui/web/src/components/tabs/SensorsTab.tsx b/gui/web/src/components/tabs/SensorsTab.tsx index 287645b..45e1f53 100644 --- a/gui/web/src/components/tabs/SensorsTab.tsx +++ b/gui/web/src/components/tabs/SensorsTab.tsx @@ -10,7 +10,7 @@ // The node is a scatter2d (R-Z points) whose `meta` carries the full per-sensor // records + the vessel outline; the backend owns every device specific (which // family is Bp vs a saddle loop, the wall shape), so this view is device-agnostic. -import { useEffect, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useState } from "react"; import type * as Plotly from "plotly.js"; import { useStore } from "../../store"; import { useNode } from "../../lib/useNode"; @@ -102,6 +102,9 @@ export default function SensorsTab({ machine }: { machine: string }) { const wallInk = dark ? "rgba(255,255,255,0.30)" : "rgba(20,34,46,0.35)"; const fluxInk = dark ? "rgba(120,170,255,0.55)" : "rgba(40,90,180,0.55)"; const vvInk = dark ? "rgba(255,255,255,0.16)" : "rgba(20,34,46,0.18)"; + // Excluded sensors: desaturated so they read as inactive against every kind's + // colour, but still clearly visible (you must be able to find and restore one). + const excludedInk = dark ? "rgba(255,255,255,0.45)" : "rgba(20,34,46,0.40)"; // 3D vessel shell — brighter/whiter than the 2D outline so it reads in the // dark scene; lighter mode bumped up slightly too. const wallSurface = dark ? "rgb(225,232,245)" : "rgb(70,90,110)"; @@ -167,6 +170,26 @@ export default function SensorsTab({ machine }: { machine: string }) { return meta.sensors.filter((s) => names.has(s.name)); }, [meta, sets, selectedSets]); + // ── bad-channel exclusions (#56) ────────────────────────────────────────── + // Click a sensor to drop it from BOTH analyses. Excluded sensors stay on the + // map, greyed, so you can see what you dropped and click it back in. + const excluded = useStore((s) => s.excluded); + const loadExclusions = useStore((s) => s.loadExclusions); + const toggleExcluded = useStore((s) => s.toggleExcluded); + const clearExclusions = useStore((s) => s.clearExclusions); + useEffect(() => { + if (machine) void loadExclusions(machine); + }, [machine, loadExclusions]); + const excludedSet = useMemo(() => new Set(excluded), [excluded]); + const onSensorClick = useCallback( + (e: Plotly.PlotMouseEvent) => { + // `text` carries the sensor name on every sensor trace (points and loops). + const name = e.points?.[0]?.text; + if (machine && typeof name === "string" && name) void toggleExcluded(machine, name); + }, + [machine, toggleExcluded], + ); + const traces2d = useMemo[]>(() => { if (!meta) return []; const t: Partial[] = []; @@ -232,36 +255,77 @@ export default function SensorsTab({ machine }: { machine: string }) { let legendShown = false; if (points.length) { - t.push({ - type: "scatter", mode: "markers", name: KIND_LABEL[kind], - legendgroup: kind, x: points.map((s) => s.r), y: points.map((s) => s.z), - text: points.map((s) => s.name), hoverinfo: "text", - marker: { size: 6, color: COLOR[kind], line: { color: wallInk, width: 0.5 } }, - } as Partial); - legendShown = true; + // Split live vs excluded so a dropped probe reads as inactive at a glance + // (grey, hollow) while staying clickable to put it back. + const live = points.filter((s) => !excludedSet.has(s.name)); + const dead = points.filter((s) => excludedSet.has(s.name)); + if (live.length) { + t.push({ + type: "scatter", mode: "markers", name: KIND_LABEL[kind], + legendgroup: kind, x: live.map((s) => s.r), y: live.map((s) => s.z), + text: live.map((s) => s.name), hoverinfo: "text", + marker: { size: 6, color: COLOR[kind], line: { color: wallInk, width: 0.5 } }, + } as Partial); + legendShown = true; + } + if (dead.length) { + t.push({ + type: "scatter", mode: "markers", name: `${KIND_LABEL[kind]} (excluded)`, + legendgroup: kind, showlegend: false, + x: dead.map((s) => s.r), y: dead.map((s) => s.z), + text: dead.map((s) => `${s.name} — excluded (click to restore)`), + hoverinfo: "text", + marker: { + size: 7, symbol: "circle-open", color: excludedInk, + line: { color: excludedInk, width: 1.5 }, + }, + } as Partial); + legendShown = true; + } } if (loops.length) { // Each loop projects onto R-Z as a segment of its poloidal length, oriented // by the sensor's own tilt (its real angle in the R-Z plane, measured from // +R toward +Z) — NOT the vessel tangent, which points off-midplane loops // the wrong way. The segment is symmetric, so tilt's sign/wrap is moot. - const x: (number | null)[] = [], y: (number | null)[] = [], txt: (string | null)[] = []; - for (const s of loops) { - const seg = loopSegment2d(s); - x.push(seg.x[0], seg.x[1], null); - y.push(seg.y[0], seg.y[1], null); - txt.push(s.name, s.name, null); + // Excluded loops go in their own greyed trace, same as the point markers. + const seg2 = (group: Sensor[]) => { + const x: (number | null)[] = [], y: (number | null)[] = [], txt: (string | null)[] = []; + for (const s of group) { + const seg = loopSegment2d(s); + x.push(seg.x[0], seg.x[1], null); + y.push(seg.y[0], seg.y[1], null); + txt.push(s.name, s.name, null); + } + return { x, y, txt }; + }; + const liveLoops = loops.filter((s) => !excludedSet.has(s.name)); + const deadLoops = loops.filter((s) => excludedSet.has(s.name)); + if (liveLoops.length) { + const { x, y, txt } = seg2(liveLoops); + t.push({ + type: "scatter", mode: "lines", name: KIND_LABEL[kind], + legendgroup: kind, showlegend: !legendShown, + x, y, text: txt, hoverinfo: "text", + line: { color: COLOR[kind], width: 2.5 }, + } as Partial); + } + if (deadLoops.length) { + const { x, y, txt } = seg2(deadLoops); + t.push({ + type: "scatter", mode: "lines", name: `${KIND_LABEL[kind]} (excluded)`, + legendgroup: kind, showlegend: false, + x, y, text: txt, hoverinfo: "text", + line: { color: excludedInk, width: 2, dash: "dot" }, + } as Partial); } - t.push({ - type: "scatter", mode: "lines", name: KIND_LABEL[kind], - legendgroup: kind, showlegend: !legendShown, - x, y, text: txt, hoverinfo: "text", - line: { color: COLOR[kind], width: 2.5 }, - } as Partial); } } return t; - }, [meta, wallInk, visibleSensors, equilibrium, fluxInk, showVV, showCoils, vvInk]); + }, [ + meta, wallInk, visibleSensors, equilibrium, fluxInk, showVV, showCoils, vvInk, + excludedSet, excludedInk, + ]); const traces3d = useMemo[]>(() => { if (!meta) return []; @@ -436,11 +500,59 @@ export default function SensorsTab({ machine }: { machine: string }) { + {/* Bad-channel exclusions (#56): what's currently dropped from every + analysis, and the way back. Only shown once something is excluded — + the hint below the plot covers discovery. */} + {excluded.length > 0 && ( +
+ {excluded.length} sensor{excluded.length === 1 ? "" : "s"} excluded + from all analysis: + {excluded.map((name) => ( + + ))} + +
+ )} + {/* No time cursor here: sensor geometry is shot-static, and the only time-dependent overlay (equilibrium) is a future backend node (#43). */}
- + +
+ Click a sensor to exclude it from all analysis (bad channel); click + again to restore. Excluded sensors stay on the map, greyed. +
diff --git a/gui/web/src/lib/api.ts b/gui/web/src/lib/api.ts index 22096e8..fd11515 100644 --- a/gui/web/src/lib/api.ts +++ b/gui/web/src/lib/api.ts @@ -87,6 +87,26 @@ export async function fetchChannelUsage(shot: string): Promise(`${API_BASE}/api/channels/${shot}`); } +/** Channels the operator has marked bad for a shot (#56). Dropped from BOTH + * analyses; still drawn (greyed) on the sensor map so you can see what went. */ +export async function fetchExclusions(shot: string): Promise { + if (!LIVE) return []; + const r = await getJSON<{ excluded: string[] }>(`${API_BASE}/api/exclusions/${shot}`); + return r.excluded ?? []; +} + +/** Replace a shot's exclusion set. Returns the stored (sorted) list. */ +export async function saveExclusions(shot: string, excluded: string[]): Promise { + if (!LIVE) throw new Error("no live backend (run the packaged app or set VITE_API_BASE)"); + const res = await fetch(`${API_BASE}/api/exclusions/${shot}`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ excluded }), + }); + if (!res.ok) throw new Error(`save failed (${res.status}): ${await res.text()}`); + return ((await res.json()) as { excluded: string[] }).excluded ?? []; +} + /** Parameters for a live shot pull (POST /api/fetch). */ export interface FetchBody { shot: number; diff --git a/gui/web/src/lib/exclusions.test.ts b/gui/web/src/lib/exclusions.test.ts new file mode 100644 index 0000000..d54dc61 --- /dev/null +++ b/gui/web/src/lib/exclusions.test.ts @@ -0,0 +1,98 @@ +// Bad-channel exclusions (#56) — the store half. +// +// The server owns the exclusion set; the store mirrors it for the Sensors view +// and bumps `excludedRev` so every open view re-fits. The parts worth pinning: +// the optimistic update greys the sensor immediately, and a FAILED save rolls +// back — a map showing an exclusion the analyses aren't honoring is a lie. +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const fetchExclusions = vi.fn(); +const saveExclusions = vi.fn(); +vi.mock("./api", () => ({ + fetchExclusions: (...a: unknown[]) => fetchExclusions(...a), + saveExclusions: (...a: unknown[]) => saveExclusions(...a), + // the store imports these at module load; never called in these tests + fetchMachines: vi.fn().mockResolvedValue([]), + fetchDevices: vi.fn().mockResolvedValue([]), + deleteMachine: vi.fn(), + deleteAllMachines: vi.fn(), +})); + +import { useStore } from "../store"; + +beforeEach(() => { + fetchExclusions.mockReset(); + saveExclusions.mockReset(); + useStore.setState({ excluded: [], excludedRev: 0 }); +}); + +describe("loadExclusions", () => { + it("mirrors the server's list for the shot", async () => { + fetchExclusions.mockResolvedValueOnce(["A", "B"]); + await useStore.getState().loadExclusions("184927"); + expect(fetchExclusions).toHaveBeenCalledWith("184927"); + expect(useStore.getState().excluded).toEqual(["A", "B"]); + }); + + it("degrades to empty with no live backend", async () => { + fetchExclusions.mockRejectedValueOnce(new Error("no backend")); + await useStore.getState().loadExclusions("1"); + expect(useStore.getState().excluded).toEqual([]); + }); +}); + +describe("toggleExcluded", () => { + it("adds a channel and persists it", async () => { + saveExclusions.mockResolvedValueOnce(["A"]); + await useStore.getState().toggleExcluded("1", "A"); + expect(saveExclusions).toHaveBeenCalledWith("1", ["A"]); + expect(useStore.getState().excluded).toEqual(["A"]); + }); + + it("removes an already-excluded channel (click to restore)", async () => { + useStore.setState({ excluded: ["A", "B"] }); + saveExclusions.mockResolvedValueOnce(["B"]); + await useStore.getState().toggleExcluded("1", "A"); + expect(saveExclusions).toHaveBeenCalledWith("1", ["B"]); + expect(useStore.getState().excluded).toEqual(["B"]); + }); + + it("adopts the server's canonical (sorted) list", async () => { + useStore.setState({ excluded: ["Z"] }); + saveExclusions.mockResolvedValueOnce(["A", "Z"]); // server sorts + await useStore.getState().toggleExcluded("1", "A"); + expect(useStore.getState().excluded).toEqual(["A", "Z"]); + }); + + it("rolls back when the save fails", async () => { + useStore.setState({ excluded: ["A"] }); + saveExclusions.mockRejectedValueOnce(new Error("500")); + await useStore.getState().toggleExcluded("1", "B"); + // never leave the map claiming an exclusion the analyses don't have + expect(useStore.getState().excluded).toEqual(["A"]); + }); + + it("bumps excludedRev so every open view re-fits", async () => { + saveExclusions.mockResolvedValueOnce(["A"]); + const before = useStore.getState().excludedRev; + await useStore.getState().toggleExcluded("1", "A"); + expect(useStore.getState().excludedRev).toBeGreaterThan(before); + }); +}); + +describe("clearExclusions", () => { + it("restores everything in one call", async () => { + useStore.setState({ excluded: ["A", "B"] }); + saveExclusions.mockResolvedValueOnce([]); + await useStore.getState().clearExclusions("1"); + expect(saveExclusions).toHaveBeenCalledWith("1", []); + expect(useStore.getState().excluded).toEqual([]); + }); + + it("rolls back a failed clear", async () => { + useStore.setState({ excluded: ["A", "B"] }); + saveExclusions.mockRejectedValueOnce(new Error("500")); + await useStore.getState().clearExclusions("1"); + expect(useStore.getState().excluded).toEqual(["A", "B"]); + }); +}); diff --git a/gui/web/src/lib/useNode.ts b/gui/web/src/lib/useNode.ts index fd9b16e..8861c20 100644 --- a/gui/web/src/lib/useNode.ts +++ b/gui/web/src/lib/useNode.ts @@ -13,6 +13,7 @@ // transient failure was unrecoverable from the UI (the QS "Plot" button with // unchanged params re-ran no effect). import { useEffect, useState } from "react"; +import { useStore } from "../store"; import { fetchNode } from "./api"; import type { Node } from "./contract"; @@ -24,7 +25,12 @@ export function useNode( params?: Record, retryKey: number = 0, ) { - const key = `${machine}::${nodeId}::${params ? JSON.stringify(params) : ""}`; + // Bad-channel exclusions (#56) live server-side and change what EVERY node + // returns, but appear in no node's params — so fold the revision counter into + // the fetch key here rather than threading it through ~30 call sites. One + // toggle in the Sensors view then re-fits every open view. + const excludedRev = useStore((s) => s.excludedRev); + const key = `${machine}::${nodeId}::${params ? JSON.stringify(params) : ""}::x${excludedRev}`; const [entry, setEntry] = useState({ key, machine, node: null, error: null }); useEffect(() => { diff --git a/gui/web/src/store.ts b/gui/web/src/store.ts index c3d18a8..d9c04fa 100644 --- a/gui/web/src/store.ts +++ b/gui/web/src/store.ts @@ -7,7 +7,9 @@ import { deleteAllMachines, deleteMachine, fetchDevices, + fetchExclusions, fetchMachines, + saveExclusions, type DeviceInfo, type MachineInfo, } from "./lib/api"; @@ -88,6 +90,12 @@ interface State { theme: Theme; fetchCreds: FetchCreds; // shared by PullControl + the QS custom-signal panel fontScale: number; + // Channels the operator marked bad for the current shot (#56). Server-side + // state mirrored here for the Sensors view; the analyses read it from the + // store on the backend, so `excludedRev` (bumped on every change) is what + // makes every open view re-fit — useNode() folds it into its fetch key. + excluded: string[]; + excludedRev: number; init: () => Promise; removeMachine: (id: string) => Promise; @@ -99,6 +107,9 @@ interface State { toggleTheme: () => void; setFetchCreds: (patch: Partial) => void; setFontScale: (n: number) => void; + loadExclusions: (shot: string) => Promise; + toggleExcluded: (shot: string, channel: string) => Promise; + clearExclusions: (shot: string) => Promise; } export const useStore = create((set) => ({ @@ -119,6 +130,8 @@ export const useStore = create((set) => ({ duoPasscode: "", }, fontScale: loadFontScale(), + excluded: [], + excludedRev: 0, async init() { // fetchDevices() guards its own errors and returns [] (no live backend / no @@ -174,6 +187,40 @@ export const useStore = create((set) => ({ applyFontScale(n); set({ fontScale: n }); }, + + // ── bad-channel exclusions (#56) ───────────────────────────────────────── + // The server is the source of truth (persisted per shot); these keep a mirror + // for the Sensors view and bump `excludedRev` so every open view re-fits. + async loadExclusions(shot) { + try { + set({ excluded: await fetchExclusions(shot), excludedRev: 0 }); + } catch { + set({ excluded: [], excludedRev: 0 }); // no live backend / no store yet + } + }, + async toggleExcluded(shot, channel) { + const cur = useStore.getState().excluded; + const next = cur.includes(channel) ? cur.filter((c) => c !== channel) : [...cur, channel]; + // Optimistic: the sensor greys immediately, then the save confirms the + // canonical (sorted) list. Roll back if the write fails so the map never + // shows an exclusion the analyses aren't actually honoring. + set((s) => ({ excluded: next, excludedRev: s.excludedRev + 1 })); + try { + const stored = await saveExclusions(shot, next); + set((s) => ({ excluded: stored, excludedRev: s.excludedRev + 1 })); + } catch { + set((s) => ({ excluded: cur, excludedRev: s.excludedRev + 1 })); + } + }, + async clearExclusions(shot) { + const cur = useStore.getState().excluded; + set((s) => ({ excluded: [], excludedRev: s.excludedRev + 1 })); + try { + await saveExclusions(shot, []); + } catch { + set((s) => ({ excluded: cur, excludedRev: s.excludedRev + 1 })); + } + }, })); // Keep the theme in sync across browser tabs: toggleTheme writes localStorage, so a diff --git a/src/magnetics/data/exclusions.py b/src/magnetics/data/exclusions.py new file mode 100644 index 0000000..7323e20 --- /dev/null +++ b/src/magnetics/data/exclusions.py @@ -0,0 +1,108 @@ +"""Per-shot bad-channel exclusions (issue #56). + +A misbehaving probe — dead channel, railed signal, wrong gain, disconnected +integrator — otherwise flows straight into both analyses and silently skews the +fits. This module is the one source of truth for "ignore these channels on this +shot", consulted below both analysis paths at channel selection so a single +exclusion applies everywhere rather than per view. + +Scope is **per shot**, because that is how bad channels actually behave: a probe +is bad *on a shot*, not universally. Exclusions persist in one small JSON file +next to the shot data (``/exclusions.json``), so they survive a reload +and are seen by anyone opening that shot on the same install. + +Transport is separate from storage: the service reads this store and the GUI +carries the resulting names on each ``/api/node`` request as an ``exclude=`` +query param. That keeps the analysis caches (all ``lru_cache`` keyed on their +arguments) correct by construction — an exclusion change is a different cache +key, so nothing stale can be served. +""" + +from __future__ import annotations + +import json +import os +import tempfile +import threading + +from . import h5source + +# One file for every shot: these are a handful of names each, and a single +# document keeps the write atomic (temp file + os.replace) without a per-shot +# file sprawl in the data dir. +_FILENAME = "exclusions.json" +_LOCK = threading.Lock() # serialize read-modify-write across request threads + + +def _path(): + return h5source.data_dir() / _FILENAME + + +def _load_all() -> dict[str, list[str]]: + try: + with open(_path(), encoding="utf-8") as fh: + raw = json.load(fh) + except (FileNotFoundError, json.JSONDecodeError, OSError): + return {} + if not isinstance(raw, dict): + return {} + # Tolerate hand-edits: keep only str->list-of-str entries. + out: dict[str, list[str]] = {} + for shot, names in raw.items(): + if isinstance(names, list): + out[str(shot)] = sorted({str(n) for n in names if str(n).strip()}) + return out + + +def get(shot: str | int) -> list[str]: + """Channel names excluded from analysis for ``shot`` (sorted, may be empty).""" + with _LOCK: + return list(_load_all().get(str(shot), [])) + + +def set_for_shot(shot: str | int, names) -> list[str]: + """Replace the exclusion set for ``shot``. Returns the stored (sorted) list. + + An empty list drops the shot's entry entirely, so a cleared shot leaves no + residue in the file. + """ + cleaned = sorted({str(n).strip() for n in (names or []) if str(n).strip()}) + with _LOCK: + all_ = _load_all() + if cleaned: + all_[str(shot)] = cleaned + else: + all_.pop(str(shot), None) + _write(all_) + return cleaned + + +def _write(all_: dict[str, list[str]]) -> None: + """Atomically replace the store (temp file in the same dir, then rename), so + a crash mid-write cannot leave a truncated file that loses every shot.""" + path = _path() + path.parent.mkdir(parents=True, exist_ok=True) + fd, tmp = tempfile.mkstemp(dir=str(path.parent), prefix=".exclusions-", suffix=".json") + try: + with os.fdopen(fd, "w", encoding="utf-8") as fh: + json.dump(all_, fh, indent=2, sort_keys=True) + fh.write("\n") + os.replace(tmp, path) + except BaseException: + try: + os.unlink(tmp) + except OSError: + pass + raise + + +def parse_param(raw) -> tuple[str, ...]: + """Parse an ``exclude=A,B`` query param into a sorted, de-duplicated tuple. + + Returned as a *tuple* because it is threaded into ``lru_cache``-keyed + analysis calls, which require a hashable argument; sorting makes the key + stable regardless of the order the GUI sent. + """ + if not raw: + return () + return tuple(sorted({p.strip() for p in str(raw).split(",") if p.strip()})) diff --git a/src/magnetics/service/app.py b/src/magnetics/service/app.py index 5f05e9e..f1a3681 100644 --- a/src/magnetics/service/app.py +++ b/src/magnetics/service/app.py @@ -30,7 +30,7 @@ from fastapi.staticfiles import StaticFiles from pydantic import BaseModel -from ..data import h5source +from ..data import exclusions, h5source from . import export, mock, nodes logger = logging.getLogger(__name__) @@ -134,6 +134,27 @@ def devices(): return out +class Exclusions(BaseModel): + """Channel names to drop from every analysis of a shot (issue #56).""" + + excluded: list[str] = [] + + +@app.get("/api/exclusions/{shot}") +def get_exclusions(shot: str): + """Channels excluded from analysis for this shot. The GUI loads these when a + shot is opened and then carries them on each /api/node request as + ``exclude=`` — storage and transport are deliberately separate so the + analysis caches stay keyed on what actually went into the fit.""" + return {"shot": shot, "excluded": exclusions.get(shot)} + + +@app.put("/api/exclusions/{shot}") +def put_exclusions(shot: str, body: Exclusions): + """Replace this shot's exclusion set (empty list clears it).""" + return {"shot": shot, "excluded": exclusions.set_for_shot(shot, body.excluded)} + + @app.get("/api/node/{shot}/{node_id}") def node(shot: str, node_id: str, request: Request): """A single bare kind-node built from real fetched shot data — the REST shape diff --git a/src/magnetics/service/nodes.py b/src/magnetics/service/nodes.py index 0e18e3c..f3065f1 100644 --- a/src/magnetics/service/nodes.py +++ b/src/magnetics/service/nodes.py @@ -20,7 +20,7 @@ import numpy as np from ..core import contracts, geometry, mode_shape, qs_bridge, spectral -from ..data import device_geom, devices, diiid_geometry, h5source +from ..data import device_geom, devices, diiid_geometry, exclusions, h5source logger = logging.getLogger(__name__) @@ -233,12 +233,34 @@ def _names(name, seen): return out +def _excluded(shot) -> tuple[str, ...]: + """Channels the operator has marked bad for this shot (issue #56), sorted. + + Read from the per-shot store rather than threaded through every signature: + this is the one place rotating channel selection happens, so reading it here + makes a single exclusion apply to *every* rotating node automatically — a new + call site cannot forget to honor it. + + Returned as a tuple so callers can pass it straight into the ``lru_cache``-d + analysis functions whose keys would not otherwise reflect the channel set + (``_spec_result``, ``_qs_run``); the rest already key on explicit ``names``. + """ + try: + return tuple(exclusions.get(str(shot))) + except Exception: # noqa: BLE001 — a broken store must not break analysis + return () + + def _array_channels(shot, families: tuple[str, ...]): """Channels present in this shot belonging to `families`, with a parseable phi, sorted by phi. Returns list of (name, phi). ``families`` are the device's - own family tokens (DIII-D pointname families, or NSTX sensor-set names).""" + own family tokens (DIII-D pointname families, or NSTX sensor-set names). + + Operator-excluded channels (#56) are dropped here, below every rotating node, + so a bad probe cannot bias an SVD / phase-vs-φ fit / n-spectrum anywhere.""" dg = _dev_geom(str(shot)) - names = h5source.channel_names(shot) + drop = set(_excluded(shot)) + names = [n for n in h5source.channel_names(shot) if n not in drop] if dg.device_id == "diiid": sel = [n for n in names if dg.family_of(n) in set(families)] else: @@ -330,6 +352,10 @@ def _geometry(shot, params=None) -> dict: if not sensors: raise ValueError("no sensors with geometry at this shot") points = [{"x": s["r"], "y": s["z"], "label": s["name"], "group": s["kind"]} for s in sensors] + # Operator-excluded channels (#56) are still returned and still drawn — the + # view greys them, so you can see what was dropped and click it back in — + # they are only removed from the analyses (see _array_channels / fit_exclude). + excluded = _excluded(shot) return contracts.scatter2d( points, {"x": "R (m)", "y": "Z (m)"}, @@ -343,6 +369,7 @@ def _geometry(shot, params=None) -> dict: "coils": geo["coils"], "arrays": geo["arrays"], "sensor_sets": geo["sensor_sets"], + "excluded": list(excluded), }, ) @@ -435,14 +462,24 @@ def _pick_pair_prefs(shot) -> tuple[tuple[str, ...], ...]: @lru_cache(maxsize=8) -def _spec_result(shot: str, slice_duration: float, coherence_smooth: int, max_columns: int = 4000): +def _spec_result( + shot: str, + slice_duration: float, + coherence_smooth: int, + max_columns: int = 4000, + exclude: tuple[str, ...] = (), +): """The (expensive) STFT, cached so the spectrogram/n-map/coherence/n-spectrum nodes share one compute. Keyed on the STFT-shaping params only; cheap post-ops (freq crop, denoise) are applied per node. Returns (result, probes, delta_phi). ``slice_duration`` sets the frequency resolution (df = 1/slice_duration) and ``max_columns`` the time-column cap (decimation lever) — the two knobs that - trade off spectrogram sharpness against compute.""" + trade off spectrogram sharpness against compute. + + ``exclude`` is not read here — ``_pick_pair`` applies it — but it MUST stay in + the signature: excluding a channel can change the probe pair, and without it + in the cache key this would serve the pre-exclusion STFT (#56).""" (n1, phi1), (n2, phi2) = _pick_pair(shot) t1, s1 = h5source.load_channel(shot, n1) t2, s2 = h5source.load_channel(shot, n2) @@ -470,7 +507,7 @@ def _prep_spec(shot, params): if cs is None: cs = _i(params, "smoothing", 5) mc = _i(params, "max_columns", 4000) - res, probes, dphi = _spec_result(str(shot), sd, max(2, cs), max(2, mc)) + res, probes, dphi = _spec_result(str(shot), sd, max(2, cs), max(2, mc), _excluded(shot)) # Optional 2-D Gaussian pre-smoothing on the FULL band, BEFORE the gates, so contiguous # coherent structure survives gating (and the display + gate share one smoothed field). # σ is in grid cells (resolution-relative); smooth_spectrogram returns a copy — the cached @@ -869,7 +906,7 @@ def _auto_freq_khz(shot, t0_ms=None, fmin=1.0, fmax=25.0): mode as the user scrubs, instead of sitting at a fixed frequency that misses it. Global peak when no cursor. Falls back to 5 kHz if the spectrogram is unavailable.""" try: - res, _probes, _dphi = _spec_result(str(shot), 0.001, 5) + res, _probes, _dphi = _spec_result(str(shot), 0.001, 5, exclude=_excluded(shot)) except Exception: # noqa: BLE001 logger.warning( "auto-freq: spectrogram unavailable for shot %s, using 5 kHz", shot, exc_info=True @@ -1502,12 +1539,17 @@ def _prep_qs_ds(shot, params): sigma_str = params.get("sigma") if params else None sigma = float(sigma_str) if sigma_str is not None else None - # fit_exclude: comma-separated exact channel names the GUI checkbox-panel has - # deselected. fit.fit matches with re.match, so anchor + escape each name for a - # literal match. Excluded channels stay in prep (still drawn on the maps); only + # fit_exclude: exact channel names dropped from the fit. Two sources, unioned: + # * the QS tab's own checkbox panel (`fit_exclude` param) — a per-fit deselect; + # * the shot's operator-marked bad channels (#56), which apply to every + # analysis, so a probe dropped in the Sensors view is dropped here too. + # fit.fit matches with re.match, so anchor + escape each name for a literal + # match. Excluded channels stay in prep (still drawn on the maps, greyed); only # the fit drops them. Sort so the tuple is a stable _qs_run cache key. excl_raw = params.get("fit_exclude", "") if params else "" - excl_names = sorted(x.strip() for x in str(excl_raw).split(",") if x.strip()) + excl_names = sorted( + {x.strip() for x in str(excl_raw).split(",") if x.strip()} | set(_excluded(shot)) + ) fit_exclude = tuple(f"{re.escape(n)}$" for n in excl_names) # Time trim: read shot-window defaults from HDF5, then apply any user override. diff --git a/tests/test_exclusions.py b/tests/test_exclusions.py new file mode 100644 index 0000000..5b2c701 --- /dev/null +++ b/tests/test_exclusions.py @@ -0,0 +1,172 @@ +"""Bad-channel exclusion (#56): the store, the API, and the analysis seam. + +A misbehaving probe must drop out of *both* analyses from one action, and must +keep rendering (greyed) on the sensor map so the operator can see what they +dropped. The subtle half is caching: several analysis functions are lru_cached, +and an exclusion that changes the answer without changing the cache key would +serve a stale pre-exclusion fit. +""" + +from __future__ import annotations + +import json + +import pytest +from fastapi.testclient import TestClient + +from magnetics.data import exclusions +from magnetics.service import app as app_mod +from magnetics.service import nodes + +from .conftest import SYNTH_SHOT + + +@pytest.fixture(autouse=True) +def _isolated_store(tmp_path, monkeypatch): + """Redirect ONLY the exclusions file to a temp dir. + + Patching ``h5source.data_dir`` would also move the shot-file lookup and hide + the synthetic fixture shot, so patch the store's own path helper instead. + """ + monkeypatch.setattr(exclusions, "_path", lambda: tmp_path / "exclusions.json") + nodes.refresh() + yield + nodes.refresh() + + +class TestStore: + def test_roundtrip_and_sorting(self): + assert exclusions.get(SYNTH_SHOT) == [] + stored = exclusions.set_for_shot(SYNTH_SHOT, ["ZZZ", "AAA", "AAA"]) + assert stored == ["AAA", "ZZZ"] # sorted + de-duplicated + assert exclusions.get(SYNTH_SHOT) == ["AAA", "ZZZ"] + + def test_blank_and_whitespace_names_are_dropped(self): + assert exclusions.set_for_shot(SYNTH_SHOT, [" A ", "", " "]) == ["A"] + + def test_clearing_removes_the_shot_entry(self, tmp_path): + exclusions.set_for_shot(SYNTH_SHOT, ["A"]) + assert exclusions.set_for_shot(SYNTH_SHOT, []) == [] + # no residue left behind for a cleared shot + assert json.loads((tmp_path / "exclusions.json").read_text()) == {} + + def test_shots_are_independent(self): + exclusions.set_for_shot(1, ["A"]) + exclusions.set_for_shot(2, ["B"]) + assert exclusions.get(1) == ["A"] + assert exclusions.get(2) == ["B"] + + def test_int_and_str_shot_ids_are_the_same_key(self): + exclusions.set_for_shot(184927, ["A"]) + assert exclusions.get("184927") == ["A"] + + def test_corrupt_store_degrades_to_empty(self, tmp_path): + (tmp_path / "exclusions.json").write_text("{not json") + assert exclusions.get(SYNTH_SHOT) == [] # never raises into the analysis + + def test_write_is_atomic_leaving_no_temp_files(self, tmp_path): + exclusions.set_for_shot(SYNTH_SHOT, ["A"]) + assert [p.name for p in tmp_path.iterdir()] == ["exclusions.json"] + + +class TestParseParam: + @pytest.mark.parametrize( + "raw,want", + [ + ("", ()), + (None, ()), + ("A", ("A",)), + ("B, A", ("A", "B")), # sorted → stable cache key + ("A,,A , ", ("A",)), # de-duplicated, blanks dropped + ], + ) + def test_parse(self, raw, want): + assert exclusions.parse_param(raw) == want + + +class TestAnalysisSeam: + """The exclusion has to bite below both analyses, not per view.""" + + def test_array_channels_drops_excluded(self): + before = nodes._array_channels(SYNTH_SHOT, ("MPI_BDOT",)) + assert before, "fixture should have a toroidal array" + victim = before[0][0] + exclusions.set_for_shot(SYNTH_SHOT, [victim]) + after = nodes._array_channels(SYNTH_SHOT, ("MPI_BDOT",)) + assert victim not in [n for n, _ in after] + assert len(after) == len(before) - 1 + + def test_every_rotating_helper_inherits_it(self): + """_toroidal_arr / _pick_pair / _toroidal_grid all resolve channels through + _array_channels, so one exclusion covers them without per-call plumbing.""" + victim = nodes._toroidal_arr(str(SYNTH_SHOT))[0][0] + exclusions.set_for_shot(SYNTH_SHOT, [victim]) + assert victim not in [n for n, _ in nodes._toroidal_arr(str(SYNTH_SHOT))] + assert victim not in [n for n, _ in nodes._pick_pair(str(SYNTH_SHOT))] + + def test_excluded_channel_still_appears_on_the_sensor_map(self): + """Dropped from the fits, still drawn — the view greys it so the operator + can see what was excluded and put it back.""" + victim = nodes._toroidal_arr(str(SYNTH_SHOT))[0][0] + exclusions.set_for_shot(SYNTH_SHOT, [victim]) + geo = nodes.build_node(str(SYNTH_SHOT), "geometry", {}) + assert victim in geo["meta"]["excluded"] + assert victim in [p["label"] for p in geo["points"]] + + def test_spectrogram_cache_is_keyed_on_exclusions(self): + """_spec_result is lru_cached without `names` in its key, but excluding a + channel can change the probe pair — the key must move or a stale + pre-exclusion STFT is served.""" + shot = str(SYNTH_SHOT) + (n1, _), (n2, _) = nodes._pick_pair(shot) + nodes._spec_result(shot, 0.002, 5, 2000, nodes._excluded(shot)) + exclusions.set_for_shot(SYNTH_SHOT, [n1]) + assert nodes._excluded(shot) == (n1,) + (m1, _), (m2, _) = nodes._pick_pair(shot) + assert n1 not in (m1, m2), "excluded probe must not be re-picked" + # different key → recomputed against the new pair, not the cached one + res, probes, _ = nodes._spec_result(shot, 0.002, 5, 2000, nodes._excluded(shot)) + assert n1 not in probes + + def test_qs_fit_exclude_unions_the_shot_exclusions(self, monkeypatch): + """The QS tab's own per-fit deselect and the shot-wide bad-channel list + must both reach the fit, not override one another.""" + seen = {} + + def _spy(*args, **kwargs): + seen["fit_exclude"] = kwargs.get("fit_exclude", args[-1] if args else ()) + raise RuntimeError("stop after capturing the cache key") + + _spy.cache_clear = lambda: None # nodes.refresh() clears the real lru_cache + monkeypatch.setattr(nodes, "_qs_run", _spy) + exclusions.set_for_shot(SYNTH_SHOT, ["BADCHAN"]) + with pytest.raises(RuntimeError): + nodes._prep_qs_ds(str(SYNTH_SHOT), {"fit_exclude": "PANELCHAN"}) + got = seen["fit_exclude"] + assert any("BADCHAN" in p for p in got), got + assert any("PANELCHAN" in p for p in got), got + + +class TestApi: + def test_get_put_roundtrip(self): + client = TestClient(app_mod.app) + r = client.get(f"/api/exclusions/{SYNTH_SHOT}") + assert r.status_code == 200 + assert r.json() == {"shot": str(SYNTH_SHOT), "excluded": []} + + r = client.put(f"/api/exclusions/{SYNTH_SHOT}", json={"excluded": ["B", "A"]}) + assert r.status_code == 200 + assert r.json()["excluded"] == ["A", "B"] + assert client.get(f"/api/exclusions/{SYNTH_SHOT}").json()["excluded"] == ["A", "B"] + + def test_put_empty_clears(self): + client = TestClient(app_mod.app) + client.put(f"/api/exclusions/{SYNTH_SHOT}", json={"excluded": ["A"]}) + assert ( + client.put(f"/api/exclusions/{SYNTH_SHOT}", json={"excluded": []}).json()["excluded"] + == [] + ) + + def test_missing_body_field_defaults_to_empty(self): + client = TestClient(app_mod.app) + assert client.put(f"/api/exclusions/{SYNTH_SHOT}", json={}).status_code == 200