diff --git a/electron/src/renderer/src/components/DpcWizard.tsx b/electron/src/renderer/src/components/DpcWizard.tsx index 6b396b57..e0b0e46f 100644 --- a/electron/src/renderer/src/components/DpcWizard.tsx +++ b/electron/src/renderer/src/components/DpcWizard.tsx @@ -65,10 +65,10 @@ const VIEW_OPTIONS: readonly { value: View; label: string }[] = [ { value: 'curl', label: 'Curl' }, ] -/** `dpc.BEAM_SHAPES` — the beam region's shape, or off. */ -type BeamShape = 'off' | 'circle' | 'ring' +/** `dpc.BEAM_SHAPES`. There is no "off": the region is always on the pattern, + * because it IS what the centre of mass is taken over. */ +type BeamShape = 'circle' | 'ring' const BEAM_SHAPES: readonly { value: BeamShape; label: string }[] = [ - { value: 'off', label: 'Off' }, { value: 'circle', label: 'Circle' }, { value: 'ring', label: 'Ring' }, ] @@ -99,7 +99,7 @@ const DEFAULTS: DpcSaved = { halfSquareWidth: 0, centerMode: 'corners', cornerFraction: 0.05, - beamShape: 'off', + beamShape: 'circle', beamCx: 0.0, beamCy: 0.0, beamR: 0.0, @@ -267,7 +267,7 @@ export function DpcWizard({ caretPos, windowId, sendAction, onClose }: Props) { // it. Same shape as the Crop / Center-Zero-Beam drag→field round trip. useWizardEvent('spyde:dpc_region', windowId, (d) => { const r: Region = { - shape: String(d.shape ?? 'off') as BeamShape, + shape: String(d.shape ?? 'circle') as BeamShape, cx: Number(d.cx ?? 0), cy: Number(d.cy ?? 0), r: Number(d.r ?? 0), r_inner: Number(d.r_inner ?? 0), brightness: d.brightness == null ? null : Number(d.brightness), @@ -433,7 +433,8 @@ export function DpcWizard({ caretPos, windowId, sendAction, onClose }: Props) { ))} - {beamShape !== 'off' && ( + {/* Always rendered: there is no shape that hides these. */} + {( <> +
Drag the {shape === 'ring' ? 'ring' : 'circle'} onto the direct beam.
) } const good = b >= 2 return ( -
{b.toFixed(1)}× frame average diff --git a/electron/tests/_harness.cjs b/electron/tests/_harness.cjs index d6f9aa65..ab31d32f 100644 --- a/electron/tests/_harness.cjs +++ b/electron/tests/_harness.cjs @@ -542,7 +542,15 @@ function backendErrorLines(backendOrLines) { // full-path colon form `dbus/bus.cc:405]` (format changed upstream, so an // Electron bump must not silently turn infrastructure noise back into // "backend errors"). - && !/:(ERROR|FATAL):[a-z_0-9/]+\.(cc|mm)[(:]\d+[)\]]/.test(l)) + && !/:(ERROR|FATAL):[a-z_0-9/]+\.(cc|mm)[(:]\d+[)\]]/.test(l) + // A CANCELLED compute, reported by the scheduler at ERROR. Any spec that + // supersedes a whole-scan pass mid-flight (drag a virtual detector, drag + // the DPC beam region) produces one per superseded pass: the client is + // still gathering the result of a graph whose keys the scheduler has just + // forgotten. It is the cancellation WORKING — the alternative, letting the + // pass run to the end, is the bug. Deliberately narrow: only the gather of + // a key in a released state, so a genuine worker loss still fails the spec. + && !/Couldn't gather keys.*'(forgotten|cancelled|released)'/.test(l)) } /** diff --git a/electron/tests/dpc_live_region.spec.ts b/electron/tests/dpc_live_region.spec.ts new file mode 100644 index 00000000..2cda0bfa --- /dev/null +++ b/electron/tests/dpc_live_region.spec.ts @@ -0,0 +1,255 @@ +/** + * dpc_live_region.spec.ts — the DPC field map must TRACK the beam region as it + * is dragged, the same way a virtual image tracks its detector ROI. + * + * Nothing headless can see this. The Python suite can prove a drag frame asks + * for a re-measure and that the frames coalesce; it cannot prove the map on + * screen actually changed, which is the entire user-visible claim. Before the + * live lane, this exact drag produced ZERO repaints across 6.4 s — the map sat + * frozen for the whole gesture and only caught up on release, because + * `_on_region_drag` armed its debounce on `pointer_up` alone. + * + * So the measurement is: sample the map's own CANVAS pixels on every drag step + * and count how many DISTINCT frames appear. Measured both ways on this drag: + * release-only scores 0, the live lane scores 12 of 24. + * + * Run: npx playwright test tests/dpc_live_region.spec.ts --project=electron \ + * --reporter=line --retries=0 + */ +import { test, expect } from '@playwright/test' +import * as fs from 'fs' +import * as path from 'path' +const { + launchApp, backendAction, waitForSubwindowCount, backendErrorLines, +} = require('./_harness.cjs') + +const SHOTS = path.join(__dirname, '..', 'dpc_live_shots') +const DPC_TITLE = /DPC Field Map/ + +/** Steps sampled during the drag. Each is one real pointer move. */ +const STEPS = 24 + +/** + * How many of those steps must show a CHANGED map. + * + * Deliberately far below `STEPS`: the pass is superseded and restarted several + * times a second, so how many repaints land inside a 120 ms sampling window is + * a property of the machine, not of the code. What is being pinned is the + * difference between "tracks the pointer" and "frozen until release", and any + * value well above zero says that. Measured: 0 release-only, 12 with the lane. + */ +const MIN_LIVE_FRAMES = 6 + +test('the DPC field map tracks the beam region while it is dragged', async () => { + test.setTimeout(600_000) + fs.mkdirSync(SHOTS, { recursive: true }) + + const ctx = await launchApp({ dask: true, env: { SPYDE_LOG_LEVEL: 'INFO' } }) + const { page, backend, assertNoJsErrors } = ctx + + try { + // LAZY + chunked, so the pass streams through the real cluster and takes + // long enough for "does it restart mid-drag?" to be answerable at all. On + // eager data every pass finishes instantly and the question is vacuous. + await backendAction(page, 'load_test_data_dpc', + { nav: 48, sig: 64, lazy: true, nav_chunk: 8 }) + await waitForSubwindowCount(page, 2, 180_000) + await page.waitForTimeout(3_000) + await page.screenshot({ path: `${SHOTS}/01-loaded.png` }) + + const sig = page.getByTestId('subwindow') + .filter({ has: page.getByTestId('subwindow-title').filter({ hasText: /S-Synthetic DPC$/ }) }) + .last() + await sig.getByTestId('subwindow-title').click() + await sig.getByTestId('subwindow-titlebar').hover() + await sig.getByTestId('action-btn-DPC').click() + await expect(sig.getByTestId('dpc-wizard')).toBeVisible({ timeout: 60_000 }) + + // The first pass has to land before we drag, or "the map changed" would be + // measuring the opening pass filling in rather than the drag. + await expect.poll( + () => sig.getByTestId('dpc-centering').getAttribute('data-worst'), + { timeout: 180_000, message: 'the opening pass never produced a field' }) + .toMatch(/\d/) + await page.screenshot({ path: `${SHOTS}/02-first-pass.png` }) + + await sig.getByTestId('dpc-tab-Center').click() + await sig.getByTestId('dpc-beam-circle').click() + await expect(sig.getByTestId('dpc-beam-r')).toBeVisible() + await page.waitForTimeout(4_000) + await page.screenshot({ path: `${SHOTS}/03-beam-on.png` }) + + const dpcWin = page.getByTestId('subwindow') + .filter({ has: page.getByTestId('subwindow-title').filter({ hasText: DPC_TITLE }) }) + .first() + await expect(dpcWin).toBeVisible({ timeout: 60_000 }) + + /** + * A fingerprint of the MAP'S OWN pixels, read off its canvases. + * + * NOT a screenshot of the window. The "Calculating…" chip sits on top of + * the figure and pulses on a 1.6 s CSS animation, so a screenshot hash + * changes on almost every sample while a pass is running — which is most of + * a drag. Measured with a window screenshot, the RELEASE-ONLY behaviour this + * spec exists to catch scored 22 of 24 "live frames" and the spec passed. A + * canvas readback sees only what the figure drew. + */ + const mapHash = async (): Promise => { + const host = await dpcWin.elementHandle() + if (!host) return 'no-window' + const parts: string[] = [] + for (const frame of page.frames()) { + const el = await frame.frameElement().catch(() => null) + if (!el) continue + const inside = await host.evaluate( + (w, f) => w.contains(f as Node), el).catch(() => false) + if (!inside) continue + parts.push(await frame.evaluate(() => { + let h = 0, n = 0 + for (const c of Array.from(document.querySelectorAll('canvas'))) { + const cv = c as HTMLCanvasElement + const g = cv.getContext('2d') + if (!g || !cv.width) continue + const d = g.getImageData(0, 0, cv.width, cv.height).data + for (let i = 0; i < d.length; i += 41) { h = (h * 31 + d[i]) | 0; n++ } + } + return `${n}:${h}` + }).catch(() => 'err')) + } + return parts.join('|') + } + + /** Where the region IS, as the backend holds it — echoed to the caret on + * every committed move. */ + const regionGeometry = async () => { + const el = sig.getByTestId('dpc-beam-readout') + const [cx, cy] = await Promise.all([ + el.getAttribute('data-cx'), el.getAttribute('data-cy'), + ]) + return cx == null || cy == null + ? null : { cx: Number(cx), cy: Number(cy) } + } + + const figureFrame = async () => { + const host = await sig.elementHandle() + if (!host) return null + for (const frame of page.frames()) { + const el = await frame.frameElement().catch(() => null) + if (!el) continue + const inside = await host.evaluate( + (w, f) => w.contains(f as Node), el).catch(() => false) + if (!inside) continue + const ok = await frame.evaluate( + () => !!(window as any).__apl_imgToCanvas + && !!(window as any)._aplTiming).catch(() => false) + if (ok) return { frame, el } + } + return null + } + + /** + * The region's centre, in PAGE coordinates. + * + * Both halves come from something authoritative rather than from the + * picture: the geometry is the backend's own and the image→canvas transform + * is anyplotlib's own (`__apl_imgToCanvas`, published for exactly this). + * + * NOT the centroid of the circle's teal pixels, which is what this did + * first. The region opens as the INSCRIBED circle, so a nudge in any + * direction runs it off the frame; the visible arc is then clipped on that + * side and its centroid moves the OTHER way. A grab check built on it reads + * "never moved" for a drag that moved perfectly well — indistinguishable + * from the bug this spec exists to catch. And aiming has to be exact + * either way: the centre hot-spot is 9 px, and a miss lands on the radius + * handle or on nothing. + */ + const centrePoint = async () => { + const g = await regionGeometry() + const f = await figureFrame() + if (!g || !f) return null + const hit = await f.frame.evaluate(([x, y]: number[]) => { + const w: any = window + for (const id of Object.keys(w._aplTiming || {})) { + const c = w.__apl_imgToCanvas(id, x, y) + if (!c) continue + // The panel's own canvas — the biggest one, since the axis strips, + // the colour bar and the status line are canvases too. The image, + // overlay and marker layers share its box, which is the box + // `__apl_imgToCanvas` returns coordinates in. + const cv = Array.from(document.querySelectorAll('canvas')) + .map((c2) => (c2 as HTMLCanvasElement).getBoundingClientRect()) + .sort((a, b) => b.width * b.height - a.width * a.height)[0] + if (!cv) return null + return { x: cv.left + c[0], y: cv.top + c[1] } + } + return null + }, [g.cx, g.cy]).catch(() => null) + if (!hit) return null + const fb = await f.el.boundingBox() + return { x: (fb?.x ?? 0) + hit.x, y: (fb?.y ?? 0) + hit.y } + } + + const sigBox = await sig.boundingBox() + const before = await regionGeometry() + // `_aplTiming` gains a panel's entry only once that panel has drawn twice, + // so on a slow machine the transform can simply not be there yet. Wait for + // it rather than reporting it as a missing circle. + let start = await centrePoint() + for (let i = 0; i < 40 && !start; i++) { + await page.waitForTimeout(500) + start = await centrePoint() + } + if (!sigBox || !before || !start) throw new Error('could not find the beam circle') + + // Prove the grab took before measuring anything (see centrePoint). + let grabbed = false + for (let attempt = 0; attempt < 4 && !grabbed; attempt++) { + await page.mouse.move(start.x, start.y) + await page.mouse.down() + await page.mouse.move(start.x, start.y + 12) + await page.waitForTimeout(400) + const now = await regionGeometry() + grabbed = !!now && Math.abs(now.cy - before.cy) > 0.5 + if (!grabbed) { + await page.mouse.up() + await page.waitForTimeout(400) + } + } + expect(grabbed, 'never managed to grab the beam circle').toBe(true) + + const seen: string[] = [await mapHash()] + for (let i = 1; i <= STEPS; i++) { + const t = i / STEPS + await page.mouse.move( + start.x + Math.sin(t * Math.PI * 2) * sigBox.width * 0.12, + start.y + t * sigBox.height * 0.18) + await page.waitForTimeout(120) + const h = await mapHash() + if (h !== seen[seen.length - 1]) seen.push(h) + } + await page.mouse.up() + const liveFrames = seen.length - 1 + await page.screenshot({ path: `${SHOTS}/04-drag-end.png` }) + + console.log(`[dpc-live] distinct map frames across ${STEPS} drag steps: ${liveFrames}`) + expect(liveFrames, + 'the field map did not repaint while the beam region was dragged — ' + + 'the re-measure is waiting for pointer_up again') + .toBeGreaterThanOrEqual(MIN_LIVE_FRAMES) + + // The region moved, so the RESTING field must differ from the one the map + // opened with. Without the trailing settle the map would be left showing + // whichever superseded partial happened to be up when the pointer stopped. + await expect.poll(mapHash, { + timeout: 90_000, + message: 'the settle never measured the resting region', + }).not.toBe(seen[seen.length - 1]) + await page.screenshot({ path: `${SHOTS}/05-after-settle.png` }) + + expect(backendErrorLines(backend), 'the backend reported an error') + .toEqual([]) + } finally { + assertNoJsErrors() + await ctx.app?.close() + } +}) diff --git a/electron/tests/dpc_workflow.spec.ts b/electron/tests/dpc_workflow.spec.ts index 29776d23..17833a01 100644 --- a/electron/tests/dpc_workflow.spec.ts +++ b/electron/tests/dpc_workflow.spec.ts @@ -366,20 +366,22 @@ test('the Center tab offers all three references, each with its own furniture', // The caret is still open from the previous test, parked on its Map tab. await page.getByTestId('dpc-tab-Center').click() - // The beam region FIRST — Manual below depends on it being on, since the - // region's centre is what Manual adopts. + // The beam region FIRST — Manual below depends on it, since the region's + // centre is what Manual adopts. // // One draggable shape that BOTH masks the centre of mass and marks the zero - // beam. Toggling Circle→Ring swaps the anyplotlib widget type, so this - // checks the shape actually changed on the PATTERN, not just in the caret. + // beam. It is ALWAYS on the pattern: there is no "off", because the region + // is what the centre of mass is taken over, and switching it off only meant + // taking the whole frame with no handle to grab. Toggling Circle→Ring swaps + // the anyplotlib widget type, so this checks the shape actually changed on + // the PATTERN, not just in the caret. const beamPixels = () => colorPixelsIn(page, sourceWindow(page), IS_BEAM) - expect(await beamPixels(), 'the region should start off').toBe(0) + await expect.poll(beamPixels, { + timeout: 30_000, message: 'the beam region was not on the pattern at open', + }).toBeGreaterThan(0) await page.getByTestId('dpc-beam-circle').click() await expect(page.getByTestId('dpc-beam-r')).toBeVisible() - await expect.poll(beamPixels, { - timeout: 30_000, message: 'the beam circle never appeared on the pattern', - }).toBeGreaterThan(0) await expect(page.getByTestId('dpc-beam-readout')) .toHaveAttribute('data-brightness', /\d/, { timeout: 30_000 }) await shot('10-beam-circle') @@ -401,14 +403,8 @@ test('the Center tab offers all three references, each with its own furniture', await page.getByTestId('dpc-info-beam').click() await expect(page.getByTestId('dpc-info-beam-text')).toHaveCount(0) - await page.getByTestId('dpc-beam-off').click() - await expect.poll(beamPixels, { - timeout: 30_000, message: 'turning the region off left its widget behind', - }).toBe(0) - // Manual — the beam region IS the marker, so this is one click, not a - // separate crosshair to place. Turn the region back on so there is a centre - // to adopt. + // separate crosshair to place. Back to a circle for it. await page.getByTestId('dpc-beam-circle').click() await expect.poll(beamPixels, { timeout: 30_000 }).toBeGreaterThan(0) await page.getByTestId('dpc-center-mode').click() diff --git a/spyde/actions/dpc.py b/spyde/actions/dpc.py index 9a4f63d1..cdd334a5 100644 --- a/spyde/actions/dpc.py +++ b/spyde/actions/dpc.py @@ -48,6 +48,7 @@ from __future__ import annotations import logging +import threading from dataclasses import dataclass, field as _dc_field import numpy as np @@ -101,7 +102,7 @@ # ───────────────────────────────────────────────────────────────────────────── #: Shapes the beam region can take. ``off`` searches the whole frame. -BEAM_SHAPES: tuple[str, ...] = ("off", "circle", "ring") +BEAM_SHAPES: tuple[str, ...] = ("circle", "ring") @dataclass @@ -127,7 +128,7 @@ class BeamRegion: widgets report and the frame ``measure_beam_shifts`` works in. Do not add a scale conversion. """ - shape: str = "off" + shape: str = "circle" cx: float = 0.0 cy: float = 0.0 r: float = 0.0 @@ -139,7 +140,15 @@ def center(self) -> tuple[float, float]: @property def active(self) -> bool: - return self.shape in ("circle", "ring") and self.r > 0 + """Whether this region can actually be integrated over. + + There is no "off" shape — the region is always on the pattern. What this + still guards is the moment BEFORE a radius exists: a sensible default is + a fraction of the detector, whose size is unknown until a dataset is + open, so a freshly constructed region carries ``r = 0`` until + :func:`default_beam_region` fills it in. + """ + return self.shape in BEAM_SHAPES and self.r > 0 def mask(self, sig_shape: tuple[int, int]) -> np.ndarray | None: """Boolean keep-mask over the detector, or ``None`` when inactive.""" @@ -170,7 +179,7 @@ def as_dict(self) -> dict: def from_dict(cls, d: dict | None) -> "BeamRegion": d = d or {} shape = str(d.get("shape", "off")) - return cls(shape=shape if shape in BEAM_SHAPES else "off", + return cls(shape=shape if shape in BEAM_SHAPES else "circle", cx=float(d.get("cx") or 0.0), cy=float(d.get("cy") or 0.0), r=float(d.get("r") or 0.0), r_inner=float(d.get("r_inner") or 0.0)) @@ -178,16 +187,30 @@ def from_dict(cls, d: dict | None) -> "BeamRegion": def default_beam_region(sig_shape: tuple[int, int], shape: str = "circle" ) -> BeamRegion: - """A region centred on the detector, generous enough to open on. - - A quarter of the shorter detector axis: comfortably larger than a typical - direct-beam disc, small enough to exclude the first-order discs that would - otherwise drag the centroid. It is a STARTING point to drag, not a - calibration — the caret shows what fraction of the frame's intensity it - captures so an obviously wrong one is visible. + """The LARGEST region that fits on the detector — the inscribed circle. + + The region is always on the pattern, so its default is what every scan is + measured with until someone drags it. That makes "generous" a correctness + requirement, not a nicety: a region that CLIPS the beam pulls the centroid + back towards its own centre and under-reads the shift. Measured on the + synthetic disc, with the region centred on the frame: + + ====================== ========================================== + radius error on a true shift of 1.5 / 3.0 / 4.5 px + ====================== ========================================== + ``0.25 * min(sy, sx)`` −0.30 / −1.00 / −1.76 px + ``0.375 * min(sy, sx)`` −0.00 / −0.00 / −0.02 px + ``0.5 * min(sy, sx)`` 0 / 0 / 0 + ====================== ========================================== + + So a quarter-frame default — chosen when the region opened OFF and only + ever appeared because someone asked for it — silently under-reads every + field by up to 40%. The inscribed circle reproduces the whole-frame answer + exactly, and shrinking it onto the beam (which is what excludes the + first-order discs) is then a deliberate act with a readout to judge it by. """ sy, sx = int(sig_shape[0]), int(sig_shape[1]) - r = max(2.0, 0.25 * min(sy, sx)) + r = max(2.0, 0.5 * min(sy, sx)) return BeamRegion(shape=shape, cx=sx / 2.0, cy=sy / 2.0, r=r, r_inner=r * 0.35) @@ -213,6 +236,61 @@ def _com_shift_frame(frame, keep, cx0: float, cy0: float): return np.array([cx0 - com_x, cy0 - com_y], dtype=np.float64) +def _region_weights(keep, sig_shape: tuple[int, int]) -> np.ndarray: + """``(3, sy, sx)`` float64: the mask, and the mask weighted by x and by y. + + STACKED, so the centre of mass is one contraction over the data rather than + three. All three quantities read the same block, and a block is the big + thing — three separate ``einsum`` calls read it three times, which measured + 73 ms against 38 ms for the stacked form on a 64x64x64x64 scan. + + Built ONCE per pass, never per frame. They are detector-sized (a few hundred + KB at most), while the data they weight is the whole scan. + """ + sy, sx = int(sig_shape[0]), int(sig_shape[1]) + mask = (np.ones((sy, sx), dtype=np.float64) if keep is None + else np.asarray(keep, dtype=np.float64)) + yy, xx = np.mgrid[0:sy, 0:sx].astype(np.float64) + return np.stack([mask, mask * xx, mask * yy]) + + +def _com_shift_blocks(block, weights, cx0: float, cy0: float) -> np.ndarray: + """Every frame in a block's ``centre − beam`` shift → ``(..., 2)``. + + The vectorised twin of :func:`_com_shift_frame`, and the one production + uses. Same arithmetic, same result — ``test_dpc.py`` asserts they agree + element for element — but as three ``einsum`` contractions over the whole + block instead of a Python call per frame with a ``scipy`` call inside it. + A scan is tens of thousands of frames, so per-frame overhead IS the runtime; + this is the same shape ``virtual_image`` reduces with, for the same reason. + + ``einsum`` also accumulates IN PLACE against detector-sized weights, so a + block's only intermediates are the nav-sized outputs. Casting the block to + float64 first would allocate four times the chunk on uint16 data — the + "spills GiBs" pathology virtual_image documents. + + Do NOT pass ``optimize=True``: it plans a contraction that materialises an + intermediate and measured 136 ms against 38 ms for the plain call. + """ + moments = np.einsum("...ij,kij->...k", block, weights) + total = moments[..., 0] + # Nothing inside the region — a NaN says "no measurement here" and + # propagates visibly, where a silent 0 would read as a centred beam. + empty = ~np.isfinite(total) | (total <= 0) + with np.errstate(invalid="ignore", divide="ignore"): + out = np.stack([cx0 - moments[..., 1] / total, + cy0 - moments[..., 2] / total], axis=-1) + out[empty] = np.nan + return out + + +#: Serialises :func:`private_view`. The copy it takes mutates the signal it is +#: copying FROM, so two threads copying one signal race — and the DPC wizard has +#: two that do: the pass lane sets a measure up while the brightness probe reads +#: a frame off the same signal. +_COPY_LOCK = threading.Lock() + + def private_view(signal): """A signal object the caller owns exclusively, over the SAME data buffer. @@ -243,12 +321,20 @@ def private_view(signal): Only the wrapper is duplicated — ``data`` is the same object, not a copy — so this stays cheap on a multi-GB lazy scan. - Call it on the DISPATCH thread, before handing the result to a worker: the - copy is itself a ``_deepcopy_with_new_data``, so it must not overlap another - operation on the signal it is copying. + Call it before handing the result to a worker, and call it for EVERY copy + of a shared signal — including the ones hyperspy takes on your behalf, which + is what ``inav`` slicing does. The copy is itself a + ``_deepcopy_with_new_data``, so two of them on one object corrupt each other + exactly as two ``map`` calls would; :data:`_COPY_LOCK` is what stops that, + and it only helps for callers that go through here. """ try: - return signal._deepcopy_with_new_data(signal.data) + # What the race looks like when it is lost: the second copy reads the + # first's half-unset object and raises `AttributeError: … has no + # attribute 'learning_results'`, which lands in the fallback below and + # hands the worker the LIVE signal — the one thing this exists to avoid. + with _COPY_LOCK: + return signal._deepcopy_with_new_data(signal.data) except Exception as e: # Worse to refuse to measure than to measure on the shared object. log.debug("private signal view failed (%s); using the live signal", e) @@ -260,9 +346,9 @@ def beam_shift_graph(signal, *, method: str = "center_of_mass", region: "BeamRegion | None" = None): """The LAZY ``(ny, nx, 2)`` beam-shift array, or ``None`` on eager data. - Split out from :func:`measure_beam_shifts` so a caller that wants the pass - to arrive progressively can hand the graph to - ``ComputeBackend.compute_chunks_progressive`` instead of blocking on it. + Split out from :func:`measure_beam_shifts` so an interactive caller can hand + the graph to ``client.compute`` and hold a CANCELLABLE future, instead of + blocking on a pass a newer beam region has already superseded. The nav chunking is preserved end to end (no rechunk layer), which is what makes per-nav-chunk streaming line up with the storage — see Live-Display §1 in CLAUDE.md and ``test_dpc.py::TestLazy``. @@ -274,12 +360,24 @@ def beam_shift_graph(signal, *, method: str = "center_of_mass", except Exception as e: # pragma: no cover log.debug("set_signal_type(electron_diffraction) failed: %s", e) if region is not None and region.active and str(method) == "center_of_mass": + import dask.array as da sy, sx = _sig_shape(signal) - out = signal.map(_com_shift_frame, keep=region.mask((sy, sx)), - cx0=sx / 2.0, cy0=sy / 2.0, - inplace=False, ragged=False, lazy_output=True, - output_signal_size=(2,), output_dtype=float, - show_progressbar=False) + data = signal.data + weights = _region_weights(region.mask((sy, sx)), (sy, sx)) + # map_blocks, not hyperspy `map`: one vectorised contraction per chunk + # rather than a Python call per frame. The nav chunking is preserved + # (no rechunk layer), which is what keeps per-nav-chunk streaming lined + # up with the storage — Live-Display §1. + return da.map_blocks( + _com_shift_blocks, data, weights=weights, + cx0=sx / 2.0, cy0=sy / 2.0, dtype=np.float64, + drop_axis=(data.ndim - 2, data.ndim - 1), + new_axis=data.ndim - 2, + chunks=data.chunks[:-2] + ((2,),), + # Declared, not probed: without it dask calls the kernel an extra + # time on a zero-sized block just to learn the output type. + meta=np.empty((0,) * (data.ndim - 1), dtype=np.float64), + ) else: kw: dict = {"method": str(method), "lazy_output": True} hw = int(half_square_width or 0) @@ -311,6 +409,7 @@ def measure_beam_shifts(signal, *, method: str = "center_of_mass", This is a REDUCTION over the dataset, not a materialisation: a lazy signal streams through ``map`` and only the ``(ny, nx, 2)`` result is computed — the memory-safety rule in CLAUDE.md is respected. + """ try: signal.set_signal_type("electron_diffraction") @@ -319,13 +418,22 @@ def measure_beam_shifts(signal, *, method: str = "center_of_mass", if region is not None and region.active and str(method) == "center_of_mass": sig_shape = _sig_shape(signal) - keep = region.mask(sig_shape) sy, sx = sig_shape - shifts = signal.map(_com_shift_frame, keep=keep, - cx0=sx / 2.0, cy0=sy / 2.0, - inplace=False, ragged=False, lazy_output=False, - output_signal_size=(2,), output_dtype=float, - show_progressbar=False) + weights = _region_weights(region.mask(sig_shape), sig_shape) + # Straight through the vectorised path — hyperspy's `map` would add a + # Python call per frame around arithmetic that is already one einsum + # over the whole array. + # + # A LAZY signal goes through `beam_shift_graph` rather than contracting + # the dask array here: this kernel's subscript is a per-BLOCK one, and + # dask's own einsum does not reproduce it. One code path for lazy data, + # and this function's contract stays a materialised (ny, nx, 2). + graph = beam_shift_graph(signal, method=method, + half_square_width=half_square_width, + region=region) + if graph is not None: + return np.asarray(graph.compute(), dtype=np.float64) + return _com_shift_blocks(signal.data, weights, sx / 2.0, sy / 2.0) else: kw: dict = {"method": str(method), "lazy_output": False} hw = int(half_square_width or 0) @@ -1191,8 +1299,6 @@ def component_titles(mode: str, units: str) -> dict[str, str]: "magnitude": f"|{sym}| ({units})", "phase": "Direction (rad)", "divergence": "Divergence", "curl": "Curl", } - - def compute_dpc(signal, *, mode: str = "magnetic", method: str = "center_of_mass", half_square_width: int = 0, center_mode: str = "corners", diff --git a/spyde/actions/dpc_action.py b/spyde/actions/dpc_action.py index c13f1955..d6bab279 100644 --- a/spyde/actions/dpc_action.py +++ b/spyde/actions/dpc_action.py @@ -27,7 +27,7 @@ instead of a click-and-wait. Do not move the measure into ``dpc_tune``. **That one pass STREAMS on lazy data.** It is dispatched per navigation chunk -through ``ComputeBackend.compute_chunks_progressive`` and the map repaints as +as ONE cancellable ``client.compute`` future, and the map repaints as each chunk lands, so a scan that takes minutes shows a field filling in rather than a spinner. Two properties make that work and are worth not breaking: @@ -88,6 +88,9 @@ import concurrent.futures import logging +import os +import threading +import time import numpy as np @@ -109,11 +112,12 @@ "half_square_width": 0, "center_mode": "corners", "corner_fraction": 0.05, - # The beam region (see BeamRegion): "off" | "circle" | "ring". It opens OFF - # so the measurement is byte-for-byte what it was before this control - # existed; the radii are filled in from the detector size the first time it - # is switched on, because a default in pixels cannot know the frame size. - "beam_shape": "off", + # The beam region (see BeamRegion): "circle" | "ring". There is no "off" — + # the region is what the centre of mass is taken over, so a pattern without + # one is just a region the size of the whole frame, drawn nowhere the user + # can grab it. The radii are filled in from the detector size once a dataset + # is open, because a default in pixels cannot know the frame size. + "beam_shape": "circle", "beam_cx": 0.0, "beam_cy": 0.0, "beam_r": 0.0, @@ -140,12 +144,6 @@ #: heavier edge than the other furniture to stay findable against it. _CORNER_LINEWIDTH = 3.0 -#: Re-measuring the whole scan is the one expensive step, so a DRAG must not -#: trigger it per frame. The widget and the readouts follow the pointer live; -#: the re-measure waits this long after motion stops. Same shape as the drift -#: caret's ROI settle, for the same reason. -_REGION_SETTLE_S = 0.45 - #: Bare-figure window geometry. A bare figure never receives ``resize_figure``, #: so its initial px size is the one it keeps and anything drawn outside is #: CLIPPED by the subwindow — see the same note in ``drift_action``. @@ -274,13 +272,20 @@ def __init__(self, session, tree, src_plot, *, params: dict | None = None): self.clim: tuple[float, float] | None = None self.cmap: str | None = None self._corner_mg = None # navigator corner boxes - self._beam_widget = None # the circle/ring on the DP - self._beam_handler = None # kept alive (weak callback) - self._beam_dragging = False # re-entrancy guard - self._settle_timer = None # drag → debounced re-measure + self._beam_selector = None # the circle/ring on the DP + self._lane = None # the serial pass-setup lane self._measure_stop: list | None = None # in-flight pass's cancel token self._measure_future = None # …and its future, if any + self._measured_region = None # the region a pass last ran + self._measure_event = None # …and what the stream waits on self._last_brightness = None # re-sent during a drag + # Guards the one hand-off `remove` and `_open_window` both reach for: + # whether this wizard is still open, and which window is its. They run + # on different threads (`_finish` lands wherever the pass did), so + # without it a pass that has already cleared its `_closed` check can + # publish a window a moment AFTER the close that was meant to prevent + # it — and `remove` has been and gone, so nothing ever closes it. + self._window_lock = threading.Lock() # ── the source signal ──────────────────────────────────────────────────── @@ -303,7 +308,7 @@ def _sig_shape(self) -> tuple[int, int]: def region(self) -> _dpc.BeamRegion: """The beam region the caret's parameters describe.""" p = self.params - return _dpc.BeamRegion(shape=str(p.get("beam_shape", "off")), + return _dpc.BeamRegion(shape=str(p.get("beam_shape", "circle")), cx=float(p.get("beam_cx") or 0.0), cy=float(p.get("beam_cy") or 0.0), r=float(p.get("beam_r") or 0.0), @@ -316,6 +321,15 @@ def measure(self, *, on_done=None) -> None: the map is repainted as each lands, so a multi-minute scan shows a field filling in rather than a spinner. On eager data (already in RAM) there is nothing to stream and it runs in one go. + + Only the BOOKKEEPING happens on the caller's thread: cancel the pass + this one replaces, take the parameters it will run with, and adopt its + cancel token so a supersede arriving a moment later still stops it. + Everything after that — copying the signal, building the graph, handing + the chunks to the backend — runs on :meth:`_pass_lane`. Measured, that + part costs 13-33 ms (hyperspy's ``map`` / ``get_direct_beam_position`` + graph build dominates), and a dragged region runs it several times a + second: inline, it stuttered the very gesture it exists to serve. """ if self.signal is None: emit_error("DPC: no active dataset") @@ -325,25 +339,23 @@ def measure(self, *, on_done=None) -> None: # waiting for costs the cluster the entire dataset. Same contract as # virtual_image (cancel prior → unregister → register new). self._cancel_measure() - # A pass gets its OWN signal object, taken here on the dispatch thread. - # The worker below runs hyperspy `map` on it, and a new pass can still - # overlap the TAIL of the one it cancels: a queued future cancels - # cleanly, one already inside `map` does not. See dpc.private_view. - signal = _dpc.private_view(self.signal) method = str(self.params["method"]) hw = int(self.params["half_square_width"] or 0) region = self.region() sig_shape = self._sig_shape() gen = self.guard() - # Only the EAGER branch below uses this one; the progressive branch owns - # its own token, because that is where a pass can actually be stopped - # part-way (see _measure_progressive). - eager_stop: list = [False] + # Adopted BEFORE the lane picks the pass up, so a supersede that lands + # while it is still queued stops it before it costs anything: the lane + # checks the token first and drops the job. During a drag that is most + # of them. + stop: list = [False] + self._track_measure(stop) + self._measured_region = region.as_dict() emit_status("DPC: locating the direct beam…") def _finish(shifts): - self._retire_measure(eager_stop) - if eager_stop[0] or not self.still(gen) or self._closed: + self._retire_measure(stop) + if stop[0] or not self.still(gen) or self._closed: return self.shifts = np.asarray(shifts, dtype=np.float64) self.report = _dpc.centering_report(self.shifts) @@ -352,31 +364,218 @@ def _finish(shifts): emit_progress(1, 1, "DPC") self.emit_state() self.refresh() + # Now, not per pointer frame: this reads a frame off the dataset. + self.emit_region(with_brightness=True) if on_done is not None: on_done() - if self._measure_progressive(signal, method, hw, region, gen, _finish): + try: + self._pass_lane().submit(self._begin_pass, stop, gen, method, hw, + region, _finish) + except RuntimeError as e: # lane shut down under a closing wizard + log.debug("DPC pass lane refused the job: %s", e) + self._retire_measure(stop) + + def _pass_lane(self): + """The ONE thread every beam-shift pass is set up on. + + Serial for the reason :func:`dpc.private_view` gives: hyperspy parks a + length-1 placeholder on ``data`` for the width of the copy, so two + threads setting a pass up on the same signal read each other's + placeholder. One lane means the copy, and the graph build after it, can + never overlap another pass's — however fast the region is dragged. + + A superseded job drops itself at the top of :meth:`_begin_pass`, so what + queues behind a slow set-up is no-ops, not stacked passes. + """ + if self._lane is None: + self._lane = concurrent.futures.ThreadPoolExecutor( + max_workers=1, thread_name_prefix="dpc-pass") + return self._lane + + def _begin_pass(self, stop, gen, method, hw, region, on_finish) -> None: + """Set the pass up and hand it to the cluster — on the pass lane. + + ONE handle, kept on ``_measure_future`` so the next move can cancel it, + and the chunks paint into the display array as they land. Both come from + ``compute_with_live_buffer``, which is what the virtual-image stream + uses: a superseded whole-scan reduction has to be CANCELLED (its + ``cancel`` stops outstanding work through the dispatcher's ``on_start`` + hook, not on a 0.5 s poll), and a scan big enough to be worth cancelling + is big enough that the user should watch it fill in. + + Data already in RAM has no graph and so no handle, and runs to the end + uninterrupted — the token can only drop its result. That is fine now + and was not before: the centre of mass is one vectorised contraction + per block rather than a Python call per frame, which measured 1429 ms → + 38 ms on a 64x64 scan of 64x64 frames. A pass that finishes in the time + of two pointer frames does not need stopping. + """ + if stop[0] or self._closed: + return + live = self.signal + if live is None: + return + try: + # Its OWN signal object: the graph runs hyperspy `map` on it, and a + # new pass can still overlap the TAIL of the one it cancels. + signal = _dpc.private_view(live) + graph = _dpc.beam_shift_graph(signal, method=method, + half_square_width=hw, region=region) + except Exception as e: + self._retire_measure(stop) + self._measure_failed(gen, e) + self._set_computing(False) + return + if stop[0] or self._closed: return - # Eager data: ONE hyperspy `map` over an array already in RAM. There is - # no interruption point inside it, so the token can only stop the pass - # BEFORE it starts and drop the result if it lands late — which is also - # all `virtual_image` does for eager data. Registering it still matters: - # closing the tree flips it, so a pass in the queue never begins. - self._track_measure(eager_stop) + client = getattr(self.tree, "client", None) + backend = getattr(self.session, "compute_backend", None) + if graph is None or (client is None and backend is None): + # Nothing to chunk and so nothing to cancel: the data is already in + # RAM, or there is neither a cluster nor a compute backend to hand + # the chunks to. The token can only drop the result. + try: + shifts = _dpc.measure_beam_shifts(signal, method=method, + half_square_width=hw, + region=region) + except Exception as e: + self._retire_measure(stop) + self._measure_failed(gen, e) + self._set_computing(False) + return + self._dispatch(lambda: on_finish(shifts)) + return - def _work(): - if eager_stop[0]: - return None - return _dpc.measure_beam_shifts(signal, method=method, - half_square_width=hw, region=region) + ny, nx = int(graph.shape[0]), int(graph.shape[1]) + field = self._display_field(signal, (ny, nx), method, hw, region, gen) + if stop[0] or self._closed: + return + self.shifts = field + self._set_computing(True) + total = max(1, len(graph.chunks[0]) * len(graph.chunks[1])) + landed = [0] + # The cluster stream waits on an Event; the backend and the tree's + # cancel registry work in `[False]` tokens. Both are set by the same + # _cancel_measure, so there is one decision and two ways of hearing it. + import threading + event = threading.Event() + # Chunks land on several pool threads at once, so the tally is taken + # under a lock: `landed[0] += 1` is three bytecodes, and a lost update + # means the count never reaches `total` — the pass then has no finish at + # all, leaving the token registered and "Calculating…" up for good. + counted = threading.Lock() - def _done(shifts): - if shifts is not None: - _finish(shifts) + def _on_chunk(chunk, slices): + """A dask callback thread — store, then marshal the repaint.""" + if stop[0] or self._closed or not self.still(gen): + return + try: + field[slices] = np.asarray(chunk, dtype=np.float64) + except Exception as e: # pragma: no cover + log.debug("storing a DPC chunk failed: %s", e) + return + with counted: + landed[0] += 1 + n = landed[0] + if n >= total: + # The last chunk IS the completed pass. Counting them is how the + # finish is known: the handle's value is assembled client-side + # from these same chunks, so waiting on it as well would only + # add a hop. + self._dispatch(lambda: on_finish(field)) + return + self._dispatch(lambda: self._on_partial(gen, n, total)) + + try: + if client is not None: + # The cluster path, and the virtual-image stream's own call: + # ONE handle, per-chunk callbacks, and a `cancel` that stops + # outstanding work through the dispatcher's `on_start` hook + # rather than on its next 0.5 s poll. + from spyde.drawing.update_functions import ( + compute_with_live_buffer) + handle = compute_with_live_buffer( + graph, (ny, nx), client, "", on_chunk_done=_on_chunk, + windowed=True, stop_event=event) + else: + # No cluster (it takes ~10 s to come up, and a scan can be open + # sooner; tests run with SPYDE_NO_DASK). The backend still + # chunks and still stops — the threaded mode checks the token + # before each submit AND inside each chunk task. Routing this + # through a plain blocking compute instead is what made a + # superseded pass run to the end. + handle = backend.compute_chunks_progressive( + graph, 2, _on_chunk, stopped_flag=stop) + except Exception as e: + self._retire_measure(stop) + self._measure_failed(gen, e) + self._set_computing(False) + return + if self._measure_stop is stop: + self._measure_event = event + self._attach_future(stop, handle) + + def _display_field(self, signal, nav_shape, method, hw, region, gen): + """The ``(ny, nx, 2)`` array the pass paints into. + + BLANK, deliberately, and this is the visible half of cancellation: the + map goes dark the instant a new pass starts and fills back in as its + chunks land, so stopping one pass and starting another is something you + can SEE rather than something you have to trust. It is what the virtual + image does, and it is the reason a superseded compute there is never in + doubt. Carrying the previous field over instead looks smoother and + hides exactly the thing worth showing. + + The FIRST pass after the caret opens is the exception: there is nothing + to supersede and nothing on screen, so it seeds from the CORNERS. Those + are a few percent of the scan, so they measure in a fraction of the time + the full pass takes, and the plane through them is the descan ramp — a + real map immediately, rather than an empty window with a spinner. + """ + ny, nx = int(nav_shape[0]), int(nav_shape[1]) + field = np.full((ny, nx, 2), np.nan, dtype=np.float64) + if self.shifts is not None: + return field # a supersede: go dark and refill + try: + corners = self._measure_corners(signal, (ny, nx), method, hw, region) + except Exception as e: + log.debug("the DPC corner seed failed: %s", e) + return field + if corners is not None and self.still(gen): + field[:] = corners + return field + + def _measure_corners(self, signal, nav_shape, method, hw, region): + """A whole-field plane fitted through the four scan corners only. + + The corners carry the instrument descan and (by assumption) none of the + sample's field, which is what makes them both cheap to measure and worth + showing on their own. + """ + fraction = float(self.params["corner_fraction"]) + sparse = np.full((int(nav_shape[0]), int(nav_shape[1]), 2), np.nan, + dtype=np.float64) + for rows, cols in _dpc.corner_slices(nav_shape, fraction): + block = _dpc.measure_beam_shifts(signal.inav[cols, rows], + method=method, + half_square_width=hw, region=region) + sparse[rows, cols] = block + return _dpc.corner_reference(sparse, fraction) + + def _dispatch(self, fn) -> None: + """Run *fn* on the event loop — figures and IPC belong there.""" + dispatch = getattr(self.session, "_dispatch_to_main", None) + dispatch(fn) if dispatch is not None else fn() + + def _on_partial(self, gen: int, done: int, total: int) -> None: + """Repaint from what has landed so far (event loop).""" + if self._closed or not self.still(gen): + return + emit_progress(done, total, "DPC: locating the direct beam") + self.refresh() - self.run_on_worker(_work, name="dpc-measure", on_done=_done, - on_error=lambda e: self._measure_failed(gen, e)) def _track_measure(self, stop: list, future=None) -> None: """Adopt *stop*/*future* as the in-flight pass and register them on the @@ -387,13 +586,34 @@ def _track_measure(self, stop: list, future=None) -> None: if reg is not None: reg(flag=stop, future=future) + def _attach_future(self, stop: list, future) -> None: + """Add the backend future to an ALREADY-adopted token. + + The token is adopted on the caller's thread; the future only exists once + the pass has been set up on the lane, by which time a newer pass may own + the slot. Attaching to a token that is no longer current would make the + next ``_cancel_measure`` cancel the WRONG pass — so if this one has been + superseded, cancel its future here instead. + """ + if self._measure_stop is not stop: + try: + if not future.done(): + future.cancel() + except Exception as e: # pragma: no cover + log.debug("cancelling a superseded DPC future failed: %s", e) + return + self._measure_future = future + reg = getattr(self.tree, "register_cancel", None) + if reg is not None: + reg(flag=stop, future=future) + def _retire_measure(self, stop: list) -> None: """Drop a FINISHED pass's token. Without this the tree's cancel registry gains an entry per measure — and every drag settle is a measure.""" if self._measure_stop is not stop: return # already superseded; not ours to drop future = self._measure_future - self._measure_stop = self._measure_future = None + self._measure_stop = self._measure_future = self._measure_event = None unreg = getattr(self.tree, "unregister_cancel", None) if unreg is not None: try: @@ -404,19 +624,30 @@ def _retire_measure(self, stop: list) -> None: def _cancel_measure(self) -> None: """Stop the in-flight beam-shift pass, if any. - Setting the flag is what actually stops it: the progressive path checks - it before each chunk submit and inside each chunk task, so a superseded - pass stops dispatching instead of computing a result nobody reads. - Cancelling the future kills one still queued; one already running ends - at its next flag check. Then unregister both, or the tree's cancel list - grows by one entry per drag. + **Cancelling the future is what stops it.** The pass is ONE + ``client.compute`` graph, so this is the same cancellation + ``virtual_image`` uses and the scheduler drops the tasks. The token + beside it covers only the window BEFORE that future exists — the graph + is built on the pass lane, and a move can land while it is — and stops a + late result being painted. Unregister both afterwards, or the tree's + cancel list grows by one entry per drag. """ stop, future = self._measure_stop, self._measure_future - self._measure_stop = self._measure_future = None + event = self._measure_event + self._measure_stop = self._measure_future = self._measure_event = None if stop is None and future is None: return if stop is not None: stop[0] = True + # The stream waits on the Event, so setting it is what stops the + # dispatcher NOW rather than on its next 0.5 s poll. + if event is not None: + event.set() + # At INFO because "is it actually cancelling, or just restarting after + # it finishes?" is the one question this path raises and the one the + # code cannot answer by inspection. + log.info("DPC: cancelled the in-flight beam-shift pass") + if future is not None: try: if not future.done(): @@ -443,100 +674,11 @@ def _measure_failed(self, gen: int, exc: Exception) -> None: if self._closed or not self.still(gen): log.debug("DPC measure abandoned after close/supersede: %s", exc) return + # With the traceback: the message alone names a symptom, and the pass + # runs on a worker, so the frames are gone by the time anyone looks. + log.exception("DPC beam-shift pass failed", exc_info=exc) emit_error(f"DPC: locating the direct beam failed: {exc}") - def _measure_progressive(self, signal, method, hw, region, gen, on_finish - ) -> bool: - """Stream the beam-shift pass per nav chunk. False → not applicable. - - Uses ``ComputeBackend.compute_chunks_progressive``, which dispatches one - task per nav chunk and calls back from a worker thread as each lands. - The lazy graph keeps the dataset's own nav chunking (no rechunk layer), - so a "chunk" here is a storage chunk — the streaming granularity matches - what the reader actually reads (Live-Display §1). - - Partial state is NaN, which every stage downstream already tolerates: - the plane fits mask on ``isfinite``, the rotation estimator drops - non-finite gradients, and the display paints non-finite black rather - than letting one poison the contrast. So the map genuinely fills in. - """ - backend = getattr(self.session, "compute_backend", None) - if backend is None or not hasattr(backend, "compute_chunks_progressive"): - return False - try: - graph = _dpc.beam_shift_graph(signal, method=method, - half_square_width=hw, region=region) - except Exception as e: - log.debug("building the DPC beam-shift graph failed: %s", e) - return False - if graph is None: # eager data — nothing to stream - return False - - ny, nx = int(graph.shape[0]), int(graph.shape[1]) - partial = np.full((ny, nx, 2), np.nan, dtype=np.float64) - total = max(1, len(graph.chunks[0]) * len(graph.chunks[1])) - done = [0] - # The cancel token travels WITH the dispatch: the backend checks it - # before each chunk submit and inside each chunk task, so superseding - # this pass stops it rather than letting it finish unread. - stop: list = [False] - self.shifts = partial - self._set_computing(True) - - def _on_chunk(chunk, slices): - """Worker thread — marshal the paint onto the main loop.""" - if stop[0] or self._closed or not self.still(gen): - return - try: - partial[slices] = np.asarray(chunk, dtype=np.float64) - except Exception as e: # pragma: no cover - log.debug("storing a DPC chunk failed: %s", e) - return - done[0] += 1 - n = done[0] - dispatch = getattr(self.session, "_dispatch_to_main", None) - paint = lambda: self._on_partial(gen, n, total) # noqa: E731 - dispatch(paint) if dispatch is not None else paint() - - try: - future = backend.compute_chunks_progressive(graph, 2, _on_chunk, - stopped_flag=stop) - except Exception as e: - log.debug("progressive DPC dispatch failed (%s) — running in one " - "pass instead", e) - return False - self._track_measure(stop, future) - - def _settled(fut): - self._retire_measure(stop) - try: - result = fut.result() - except concurrent.futures.CancelledError: - # Cancelled deliberately (superseded, or the caret/tree closed). - # Not a failure and not the user's problem — say nothing. - self._set_computing(False) - return - except Exception as e: - if stop[0]: # torn down mid-pass; same story - self._set_computing(False) - return - self._measure_failed(gen, e) - self._set_computing(False) - return - dispatch = getattr(self.session, "_dispatch_to_main", None) - finish = lambda: on_finish(result) # noqa: E731 - dispatch(finish) if dispatch is not None else finish() - - future.add_done_callback(_settled) - return True - - def _on_partial(self, gen: int, done: int, total: int) -> None: - """Repaint from what has landed so far (main thread).""" - if self._closed or not self.still(gen): - return - emit_progress(done, total, "DPC: locating the direct beam") - self.refresh() - def _set_computing(self, computing: bool) -> None: """Drive the window's "Calculating…" overlay. Every True is paired.""" try: @@ -642,6 +784,8 @@ def derive(self) -> _dpc.DpcResult | None: def refresh(self) -> None: """Derive and repaint the map (opening the window on the first call).""" + if self._closed: + return result = self.derive() if result is None: return @@ -665,12 +809,17 @@ def _open_window(self, result: _dpc.DpcResult) -> None: emit_error(f"DPC: building the result window failed: {e}") log.exception("DPC window build failed") return - wid = int(self.session.next_window_id()) - keep_alive(wid, fig) - self.window_id, self.plot, self.wheel = wid, plot, wheel - emit({"type": "figure", "fig_id": fig_id, "window_id": wid, - "html": html, "title": self._title(), "is_navigator": False, - "aspect": _FIG_WIDTH / float(_FIG_HEIGHT)}) + # Claim and publish together: `remove` may have run while the figure + # was being built, and a window emitted after it is one nobody owns. + with self._window_lock: + if self._closed: + return + wid = int(self.session.next_window_id()) + keep_alive(wid, fig) + self.window_id, self.plot, self.wheel = wid, plot, wheel + emit({"type": "figure", "fig_id": fig_id, "window_id": wid, + "html": html, "title": self._title(), "is_navigator": False, + "aspect": _FIG_WIDTH / float(_FIG_HEIGHT)}) self.own_window(wid) #: The live window's title. Deliberately does NOT name the field type. @@ -794,160 +943,191 @@ def hide_corner_boxes(self) -> None: # ── the beam region (one shape, two jobs) ──────────────────────────────── def ensure_region_defaults(self) -> None: - """Fill in radii the first time the region is switched on. + """Fill in the radii the first time a dataset is open. They cannot be declared in ``DEFAULTS`` because a sensible radius is a - fraction of the DETECTOR, whose size is not known until a dataset is - open. + fraction of the DETECTOR, whose size is not known until then — which is + also why ``BeamRegion.active`` still exists with the "off" shape gone. """ p = self.params - if str(p.get("beam_shape", "off")) == "off" or float(p.get("beam_r") or 0) > 0: + if float(p.get("beam_r") or 0) > 0: return d = _dpc.default_beam_region(self._sig_shape(), str(p["beam_shape"])) p["beam_cx"], p["beam_cy"] = d.cx, d.cy p["beam_r"], p["beam_r_inner"] = d.r, d.r_inner - def show_beam_region(self) -> None: - """Draw (or reshape) the draggable circle / ring on the pattern. + def sync_beam_region(self) -> None: + """Put the selector for the current shape on the pattern. - The SAME widget answers both of the Center step's questions: its area is - the centre-of-mass mask, and its centre is the Manual reference. Two - separate controls for one physical thing (where is the beam?) is what - this replaces. + The beam region is a :class:`~spyde.drawing.selectors.CircleSelector` / + :class:`~spyde.drawing.selectors.AnnularSelector` — the same selectors a + virtual image puts on the same plot — so it inherits the whole live + path: every pointer frame submits to the one serial navigator + dispatcher, a newer position replaces a still-queued older one, and a + trailing settle re-fires once motion stops. It used to be a raw widget + with a hand-rolled debounce, which is the only reason the map did not + track the region the way a virtual image tracks its detector ROI. - Switching shape rebuilds the widget — a circle and an annulus are - different anyplotlib widget types, so there is nothing to mutate. + Switching shape REBUILDS it — a circle and an annulus are different + anyplotlib widget types, so there is nothing to mutate. """ - plot2d = getattr(self.src_plot, "_plot2d", None) - shape = str(self.params.get("beam_shape", "off")) - if plot2d is None or shape == "off": - self.hide_beam_region() + from spyde.drawing.selectors import AnnularSelector, CircleSelector + + if self._closed or getattr(self.src_plot, "_plot2d", None) is None: return self.ensure_region_defaults() - r = self.region() - if self._beam_widget is not None: - if getattr(self._beam_widget, "_dpc_shape", None) == shape: - try: - kw = ({"cx": r.cx, "cy": r.cy, "r_outer": r.r, - "r_inner": r.r_inner} if shape == "ring" - else {"cx": r.cx, "cy": r.cy, "r": r.r}) - self._beam_widget.set(**kw) - return - except Exception as e: # pragma: no cover - log.debug("resizing the DPC beam region failed: %s", e) + region = self.region() + shape = region.shape + selector = self._beam_selector + if selector is not None and getattr(selector, "_dpc_shape", "") != shape: self.hide_beam_region() + selector = None + if selector is None: + cls = AnnularSelector if shape == "ring" else CircleSelector + try: + # No children: this selector drives a whole-scan re-measure, not + # a sliced child plot, so the work hangs off `index_hooks` — the + # same seam the vector overlays use. `_run_update`'s child loop + # is then a no-op and its geometry de-duplication still applies. + selector = cls(parent=self.src_plot, children=[], + update_function=[], color=_BEAM_COLOR) + except Exception as e: # pragma: no cover + log.debug("building the DPC beam selector failed: %s", e) + return + selector._dpc_shape = shape + selector.index_hooks.append(self._on_region_moved) + self._beam_selector = selector + self._write_region_to_widget(region) + + def _write_region_to_widget(self, region) -> None: + """Push *region* onto the widget (a typed radius, a shape rebuild). + + Safe to call from anywhere: unlike the old raw widget this does NOT + re-enter synchronously. ``set`` fires ``pointer_move``, but the selector + answers that by submitting to the dispatcher, so the read-back lands on + another thread and one write cannot recurse into another. + + Then push the PANEL as well, which is not redundant. ``set`` reaches the + figure as a targeted widget message and deliberately never rewrites the + panel's own state (anyplotlib says so on ``Figure._push_widget``, and + reconciles the same divergence in ``_sync_for_export`` before any + snapshot). The renderer RETAINS the last panel state per figure and + re-applies it — so until something else repaints this plot, the geometry + the figure falls back to is the one the widget was BORN with. For the + beam region that is ``image_width * 0.1``, a fifth of the radius it + opens at: the circle silently collapsed to it the first time the pointer + entered the pattern, and the drag that followed moved that collapsed + circle. Pushing here makes the panel state agree with the widget, so + there is nothing stale to fall back to. + """ + widget = getattr(self._beam_selector, "roi", None) + if widget is None: + return try: - if shape == "ring": - w = plot2d.add_annular_widget(cx=r.cx, cy=r.cy, r_outer=r.r, - r_inner=r.r_inner, - color=_BEAM_COLOR) - else: - w = plot2d.add_circle_widget(cx=r.cx, cy=r.cy, r=r.r, - color=_BEAM_COLOR) - w._dpc_shape = shape - from spyde.drawing.selectors.base_selector import event_handler_fn - handler = event_handler_fn(lambda event: self._on_region_drag(event)) - w.add_event_handler(handler, "pointer_move", "pointer_up") - self._beam_widget, self._beam_handler = w, handler + widget.set(**({"cx": region.cx, "cy": region.cy, + "r_outer": region.r, "r_inner": region.r_inner} + if region.shape == "ring" + else {"cx": region.cx, "cy": region.cy, "r": region.r})) + plot2d = getattr(widget, "_plot", None) + if plot2d is not None: + plot2d._push() except Exception as e: # pragma: no cover - log.debug("adding the DPC beam region failed: %s", e) + log.debug("writing the DPC beam region to its widget failed: %s", e) def hide_beam_region(self) -> None: - """Widgets have no ``remove()``, only ``hide()`` (same as CZB's).""" - if self._beam_widget is not None: - try: - self._beam_widget.hide() - except Exception as e: # pragma: no cover - log.debug("hiding the DPC beam region failed: %s", e) - self._beam_widget = self._beam_handler = None - - def _on_region_drag(self, event=None) -> None: - """Read the widget back, echo its geometry, and on RELEASE re-measure. - - Everything expensive waits for ``pointer_up``. A drag frame only reads - the widget and echoes numbers the caret already has in hand. - - The brightness readout is what makes this necessary. It reads a frame, - which on a lazy signal is a dask compute, and one per pointer frame - queues work faster than it drains — the caret's own radius then keeps - climbing for a while after the pointer stops, because the messages - behind it are still landing. Same reason the Fit caret sends its state - on release only (see ``background_action._on_window_drag``). - - RE-ENTRANCY GUARD: anyplotlib ``Widget.set()`` fires ``pointer_move`` - unconditionally — even on a no-change write — so anything here that - writes back to the widget re-invokes this handler synchronously. The - same recursion Crop and CZB both hit; compare-before-set is NOT enough. + """Take the selector off the pattern (shape change, or teardown).""" + selector = self._beam_selector + self._beam_selector = None + if selector is None: + return + try: + selector.close() + except Exception as e: # pragma: no cover + log.debug("closing the DPC beam selector failed: %s", e) + + def _on_region_moved(self, indices=None) -> None: + """The region moved — track it and re-measure. On the dispatcher thread. + + This is the virtual-image update function's job, in the virtual-image + update function's place: the selector fires it once per COMMITTED + position (repeats at the same geometry are de-duplicated, and a + superseded position was dropped from the pending slot before it ever + ran), and it cancels the compute it replaces and starts a new one. + There is no pacing on top of that — the dispatcher's latest-wins + coalescing IS the pacing, exactly as it is for a virtual image. """ - w = self._beam_widget - if w is None or self._beam_dragging or self._closed: + if self._closed: + return + widget = getattr(self._beam_selector, "roi", None) + if widget is None: return - self._beam_dragging = True try: - self.params["beam_cx"] = float(w.cx) - self.params["beam_cy"] = float(w.cy) + self.params["beam_cx"] = float(widget.cx) + self.params["beam_cy"] = float(widget.cy) if str(self.params.get("beam_shape")) == "ring": - self.params["beam_r"] = float(w.r_outer) - self.params["beam_r_inner"] = float(w.r_inner) + self.params["beam_r"] = float(widget.r_outer) + self.params["beam_r_inner"] = float(widget.r_inner) else: - self.params["beam_r"] = float(w.r) + self.params["beam_r"] = float(widget.r) except Exception as e: # pragma: no cover log.debug("reading the DPC beam region failed: %s", e) - finally: - self._beam_dragging = False - released = str(getattr(event, "event_type", "") or "") == "pointer_up" - self.emit_region(with_brightness=released) - if released: - self.arm_region_settle() - - def arm_region_settle(self) -> None: - """(Re)start the debounce that re-measures once the drag stops. - - The region changes the centre of mass, so it can only take effect by - re-measuring the whole scan — the one expensive step. Doing that per - drag frame would make the widget unusable on any real dataset, so the - widget and the brightness readout track the pointer and the measurement - follows once motion stops. - """ - import threading - if self._settle_timer is not None: - try: - self._settle_timer.cancel() - except Exception as e: # pragma: no cover - log.debug("cancelling the DPC settle timer failed: %s", e) - if self._closed: return - - def _fire(): - self._settle_timer = None - if self._closed: - return - dispatch = getattr(self.session, "_dispatch_to_main", None) - if dispatch is not None: - dispatch(self.measure) - else: - self.measure() - - self._settle_timer = threading.Timer(_REGION_SETTLE_S, _fire) - self._settle_timer.daemon = True - self._settle_timer.start() + # Geometry only: the brightness readout reads a FRAME, and one dask + # compute per pointer frame queues work faster than it drains however + # far off-thread it runs. It is refreshed when the pass lands. + self.emit_region(with_brightness=False) + if self.region().as_dict() == self._measured_region: + # The region the running (or last) pass already used. Reached on + # every OPEN, because placing the selector writes its geometry and + # the widget reports that write as a move — which superseded the + # opening pass with an identical one, throwing away a whole scan's + # work and the progressive fill with it. Also covers a drag that + # returns to where it started. + return + self.measure() def emit_region(self, with_brightness: bool = True) -> None: """Live region geometry + how bright it is, for the caret's readout. - ``with_brightness=False`` re-sends the last measured value instead of - reading a frame for a new one. Mid-drag the geometry is what the caret - needs to track the pointer; the brightness costs a frame read and is - recomputed on release. Re-sending the last value rather than ``None`` - keeps the readout from blanking on every drag. + The geometry goes out NOW, carrying whatever brightness was last + measured — the caret has to track the pointer, and re-sending the last + value rather than ``None`` keeps the readout from blanking on every + drag. ``with_brightness`` additionally asks for a fresh reading, which + costs a frame read (a dask compute on a lazy signal), so it happens on a + worker and lands in a second message. Doing it inline stalled the event + loop on exactly the gesture that must not stall. """ region = self.region() + self._send_region(region) signal = self.signal - if with_brightness: - brightness = (_dpc.region_brightness(signal, region) - if signal is not None else float("inf")) + if not with_brightness or signal is None: + return + # Its OWN view, for the reason `private_view` gives: the probe reads one + # frame with `inav`, and hyperspy takes an `inav` slice by deep-copying + # the signal it slices. Probing the LIVE signal on a worker therefore + # races the pass lane copying that same signal — and the loser of that + # race falls back to measuring on the shared object. + probe = _dpc.private_view(signal) + + def _work(): + return _dpc.region_brightness(probe, region) + + def _done(brightness): + # Drop a reading for a region the user has already moved on from — + # a stale multiplier next to a region it does not describe reads as + # a wrong answer, not a late one. + if self._closed or self.region().as_dict() != region.as_dict(): + return self._last_brightness = (None if not np.isfinite(brightness) else float(brightness)) + self._send_region(region) + + self.run_on_worker(_work, name="dpc-brightness", on_done=_done, + on_error=lambda e: log.debug( + "the DPC brightness probe failed: %s", e)) + + def _send_region(self, region) -> None: + """One ``dpc_region`` message for *region* + the last known brightness.""" emit({"type": "dpc_region", "window_id": self.caret_window_id, "result_window_id": self.window_id, **region.as_dict(), @@ -957,15 +1137,14 @@ def sync_overlays(self) -> None: """Show exactly the furniture the current state needs. The corner boxes belong to one Center MODE; the beam region does not — - it defines the centre of mass for every mode, so it is shown whenever - it is switched on. + it defines the centre of mass for every mode, so it is always there. """ mode = str(self.params["center_mode"]) if mode == "corners": self.show_corner_boxes() else: self.hide_corner_boxes() - self.show_beam_region() + self.sync_beam_region() # ── rotation ───────────────────────────────────────────────────────────── @@ -1156,7 +1335,7 @@ def _attach_wheel(tree): return commit_result_tree( self.session, title=f"DPC ({sym})", # The primary is the RGB direction+magnitude image, so label it that - # way — calling it "Ex" put a chip next to the real "Ex (MV/cm)" + # way — calling it "Ex" put a chip next to the real "Ex (MV/cm)" # view claiming to be the same map. primary=r.rgb, primary_label=f"{sym} direction", views=[(titles[c], r.component(c)) for c in _dpc.COMPONENTS], @@ -1174,43 +1353,50 @@ def _attach_wheel(tree): on_tree=_attach_wheel, ) - # ── teardown ───────────────────────────────────────────────────────────── + # ── teardown ───────────────────────────────────────────────────────────── def remove(self) -> None: """Tear down everything the wizard added. Idempotent — re-entry through remove → _forget_window → close → remove is a no-op.""" - if self._closed: - return - self._closed = True + # Claim the close and TAKE the window in one step, under the lock + # `_open_window` publishes through: a pass landing on another thread + # either sees the close and opens nothing, or gets in first and leaves + # a `window_id` here to be closed. Nothing between the two. + with self._window_lock: + if self._closed: + return + self._closed = True + window_id = self.window_id + self.window_id = self.plot = self.wheel = None # Stop the in-flight pass. Closing the caret is the clearest case of # "nobody is waiting for this any more", and a beam-shift pass reads the # whole scan — it must not keep running for a wizard that is gone. self._cancel_measure() - # Cancel the drag debounce FIRST: a timer that fires after teardown - # would re-measure a torn-down wizard on a worker thread. - if self._settle_timer is not None: - try: - self._settle_timer.cancel() - except Exception as e: # pragma: no cover - log.debug("cancelling the DPC settle timer failed: %s", e) - self._settle_timer = None + # The selector owns the only remaining drag timer (its settle re-fire) + # and cancels it in `close`; `_closed` is already True, so a hook that + # fires in the meantime bails out on its own. self.hide_corner_boxes() self.hide_beam_region() - if self.window_id is not None: + if window_id is not None: forget = getattr(self.session, "_forget_window", None) if forget is not None: try: - forget(int(self.window_id)) + forget(int(window_id)) except Exception as e: # pragma: no cover log.debug("forgetting the DPC window failed: %s", e) else: # pragma: no cover - emit({"type": "window_closed", "window_id": int(self.window_id)}) + emit({"type": "window_closed", "window_id": int(window_id)}) reg = getattr(self.session, "_window_controllers", None) if isinstance(reg, dict): - reg.pop(int(self.window_id), None) - self.window_id = self.plot = self.wheel = None + reg.pop(int(window_id), None) if getattr(self.tree, "_dpc_wizard", None) is self: self.tree._dpc_wizard = None + # Last, and without waiting: a job still on the lane has already been + # stopped by the cancel above and returns on its next token check, but + # `remove` runs on the event loop and must not block on it. + lane, self._lane = self._lane, None + if lane is not None: + lane.shutdown(wait=False) def _tree_title(tree) -> str: @@ -1218,8 +1404,6 @@ def _tree_title(tree) -> str: return str(tree.root.metadata.General.title) or "untitled" except Exception: # pragma: no cover return "untitled" - - # ── toolbar entry (ActionContext convention: fn(ctx, ...)) ──────────────────── def dpc(ctx, action_name: str = "DPC", **params) -> None: @@ -1320,24 +1504,21 @@ def dpc_set_center(session, plot, payload) -> None: def dpc_set_beam(session, plot, payload) -> None: - """Beam region: switch between off / circle / ring, or set its geometry. + """Beam region: switch circle/ring, or type a radius. - Changing the region changes the CENTRE OF MASS, so it can only take effect - by re-measuring — which this does immediately for a discrete change (a - shape toggle, a typed radius). A DRAG goes through the debounce instead - (``_on_region_drag``), because the whole scan cannot be re-measured per - pointer frame. + This only writes the geometry onto the widget. The re-measure follows from + the widget the same way it does for a drag — ``sync_beam_region`` pushes the + value, the selector reports the move, and ``_on_region_moved`` decides. One + path for "the region changed", whether a pointer or a number moved it; + measuring here as well would run the pass twice for every typed radius. """ ctrl = _ctrl_for(session, plot, payload) if ctrl is None: return - before = ctrl.region().as_dict() ctrl.params.update(_clean(payload)) ctrl.ensure_region_defaults() ctrl.sync_overlays() - ctrl.emit_region() - if ctrl.region().as_dict() != before: - ctrl.measure() + ctrl.emit_region(with_brightness=False) def dpc_pick_center(session, plot, payload) -> None: diff --git a/spyde/tests/migrated/test_dpc.py b/spyde/tests/migrated/test_dpc.py index 886f87fc..84bfacbd 100644 --- a/spyde/tests/migrated/test_dpc.py +++ b/spyde/tests/migrated/test_dpc.py @@ -442,16 +442,39 @@ def test_an_explicit_center_still_wins_over_the_region(self): def test_region_round_trips_through_a_dict(self): r = dpc.BeamRegion("ring", 1.5, 2.5, 9.0, 3.0) assert dpc.BeamRegion.from_dict(r.as_dict()) == r - assert dpc.BeamRegion.from_dict(None).shape == "off" - assert dpc.BeamRegion.from_dict({"shape": "nonsense"}).shape == "off" - - def test_default_region_is_a_sane_starting_point(self): + assert dpc.BeamRegion.from_dict(None).shape == "circle" + assert dpc.BeamRegion.from_dict({"shape": "nonsense"}).shape == "circle" + assert dpc.BeamRegion.from_dict({"shape": "off"}).shape == "circle", \ + "a saved report from before the region was always-on must still load" + + def test_default_region_is_the_largest_that_fits(self): + """It must not CLIP the beam. The region is always on the pattern, so + this default is what an unattended scan is measured with, and a smaller + circle pulls the centroid towards its own centre — see + ``default_beam_region`` for the measured under-read.""" d = dpc.default_beam_region((256, 512)) assert (d.cx, d.cy) == (256.0, 128.0) # centred on the DETECTOR - assert d.r == 64.0 # a quarter of the short axis + assert d.r == 128.0 # HALF the short axis assert 0 < d.r_inner < d.r assert d.active + def test_the_default_region_does_not_bias_the_shift(self): + """The claim the radius rests on, checked rather than asserted: on a + centred disc displaced by a known amount, the default region returns the + SAME shift the whole frame does.""" + from scipy import ndimage as ndi + k = 32 + gy, gx = np.mgrid[0:k, 0:k].astype(np.float32) + region = dpc.default_beam_region((k, k)) + keep = region.mask((k, k)) + for true_shift in (1.5, 3.0, 4.5): + r = np.hypot(gx - (k / 2 - true_shift), gy - k / 2) + frame = np.clip((k * 0.22 - r) / (k * 0.05) + 0.5, 0, 1) + masked = dpc._com_shift_frame(frame, keep, k / 2, k / 2) + whole_cy, whole_cx = ndi.center_of_mass(frame) + assert masked[0] == pytest.approx(k / 2 - whole_cx, abs=0.02) + assert masked[0] == pytest.approx(true_shift, abs=0.05) + def test_the_region_lands_in_provenance(self): s, _k = self._scan() region = dpc.BeamRegion("ring", 40.0, 36.0, 12.0, 3.0) @@ -899,29 +922,87 @@ def test_eager_data_has_no_graph(self): s.set_signal_type("electron_diffraction") assert dpc.beam_shift_graph(s) is None - def test_the_kernel_only_ever_sees_one_frame(self): - """Streaming is per FRAME inside a chunk, so peak memory is a frame — - not a chunk, and certainly not the dataset.""" + def test_the_kernel_sees_one_chunk_and_allocates_only_the_result(self): + """Peak memory is a chunk — never the dataset — and the kernel is + VECTORISED over that chunk rather than called per frame. + + It used to be a Python call per frame with a ``scipy.ndimage`` call + inside it, which is why a pass over a real scan took most of a minute: + at tens of thousands of frames the per-call overhead IS the runtime. + One ``einsum`` per chunk measured 37x faster on a 64x64x64x64 scan and + returns bit-identical numbers (see + ``test_the_vectorised_kernel_matches_the_per_frame_reference``). + + What still has to hold is the memory rule: the kernel must not + materialise a float64 copy of the block it is handed. It contracts the + block against detector-sized weights instead, so the only thing it + allocates is the nav-sized result. + """ s, arr = self._lazy() - seen = {"max": 0, "n": 0} - real = dpc._com_shift_frame + seen = {"blocks": [], "n": 0} + real = dpc._com_shift_blocks - def spy(frame, **kw): + def spy(block, *a, **kw): seen["n"] += 1 - seen["max"] = max(seen["max"], np.asarray(frame).nbytes) - return real(frame, **kw) + seen["blocks"].append(np.asarray(block).nbytes) + out = real(block, *a, **kw) + assert out.nbytes < np.asarray(block).nbytes, \ + "the kernel returned something the size of its input" + return out - dpc._com_shift_frame = spy + dpc._com_shift_blocks = spy try: shifts = dpc.measure_beam_shifts( s, region=dpc.BeamRegion("circle", 18.0, 14.0, 8.0)) finally: - dpc._com_shift_frame = real - one_frame = arr.shape[2] * arr.shape[3] * arr.dtype.itemsize - assert seen["n"] == arr.shape[0] * arr.shape[1] - assert seen["max"] <= one_frame * 1.01 + dpc._com_shift_blocks = real + + chunk_bytes = max( + np.prod([c[0] for c in arr.chunks]) * arr.dtype.itemsize, 1) + assert seen["n"] == len(arr.chunks[0]) * len(arr.chunks[1]), \ + "the kernel ran per frame, not per chunk" + assert max(seen["blocks"]) <= chunk_bytes * 1.01, \ + "the kernel was handed more than one chunk" assert shifts.shape == (arr.shape[0], arr.shape[1], 2) + def test_the_vectorised_kernel_matches_the_per_frame_reference(self): + """The fast path and the reference must agree EXACTLY, not closely. + + ``_com_shift_frame`` is the documented equivalence to pyxem's + ``center_of_mass_from_image`` (``TestBeamRegion`` pins that), so the + vectorised kernel inherits that guarantee only for as long as the two + return the same bits. ``allclose`` would let a real drift through. + """ + rng = np.random.default_rng(0) + k = 16 + block = (rng.random((3, 4, k, k)) * 1000).astype(np.uint16) + region = dpc.BeamRegion("circle", k / 2, k / 2, k * 0.4) + keep = region.mask((k, k)) + + fast = dpc._com_shift_blocks( + block, dpc._region_weights(keep, (k, k)), k / 2, k / 2) + for iy in range(block.shape[0]): + for ix in range(block.shape[1]): + one = dpc._com_shift_frame(block[iy, ix], keep, k / 2, k / 2) + assert np.array_equal(fast[iy, ix], one), (iy, ix) + + def test_an_empty_region_reads_as_no_measurement(self): + """A region with nothing under it yields NaN, not a centred beam. + + Zero is the dangerous answer: it is exactly what an undeflected beam + looks like, so it would show as a real measurement of no field. + """ + k = 16 + block = np.zeros((2, 2, k, k), dtype=np.uint16) + block[1, 1, k // 2, k // 2] = 500 # one frame has signal + region = dpc.BeamRegion("circle", k / 2, k / 2, k * 0.4) + out = dpc._com_shift_blocks( + block, dpc._region_weights(region.mask((k, k)), (k, k)), + k / 2, k / 2) + assert np.isnan(out[0, 0]).all(), "an empty region reported a position" + assert np.isfinite(out[1, 1]).all(), "a real frame was dropped" + + def test_a_partial_field_survives_every_downstream_stage(self): """The whole point of streaming: NaN where nothing has landed yet must flow through centering, rotation and display without poisoning them.""" diff --git a/spyde/tests/migrated/test_dpc_action.py b/spyde/tests/migrated/test_dpc_action.py index eb7ecfcf..b61079cc 100644 --- a/spyde/tests/migrated/test_dpc_action.py +++ b/spyde/tests/migrated/test_dpc_action.py @@ -32,6 +32,7 @@ """ from __future__ import annotations +import threading import time import numpy as np @@ -270,23 +271,23 @@ def test_switching_mode_takes_the_previous_furniture_away(self, window): beam_shape="circle") nav = _navigator_plot(session) assert "dpc_corners" in _markers(nav._plot2d) - assert wiz._beam_widget is not None + assert wiz._beam_selector.roi is not None dpca.dpc_set_center(session, plot, {"center_mode": "manual"}) assert wiz._corner_mg is None assert "dpc_corners" not in _markers(nav._plot2d) - assert wiz._beam_widget is not None, \ + assert wiz._beam_selector.roi is not None, \ "the beam region is not owned by a Center mode" dpca.dpc_set_center(session, plot, {"center_mode": "none"}) - assert wiz._corner_mg is None and wiz._beam_widget is not None + assert wiz._corner_mg is None and wiz._beam_selector.roi is not None def test_the_region_centre_becomes_the_manual_reference(self, window): """Drag the region onto the beam and Manual is already answered — no second marker to place, and no way for the two to disagree.""" session, plot, _tree, wiz = _opened(window, center_mode="manual", beam_shape="circle") - assert wiz._beam_widget is not None + assert wiz._beam_selector.roi is not None wiz.params.update({"beam_cx": 20.0, "beam_cy": 12.0, "beam_r": 6.0}) dpca.dpc_pick_center(session, plot, {}) assert wiz.params["cx"] == 20.0 and wiz.params["cy"] == 12.0 @@ -305,9 +306,12 @@ def test_the_reference_follows_the_region_without_an_explicit_pick(self, window) assert wiz.manual_center() == (18.0, 14.0) assert wiz.reference()[..., 0] == pytest.approx(16.0 - 18.0) - def test_picking_with_no_region_errors_instead_of_guessing(self, window): - session, plot, _tree, _wiz = _opened(window, center_mode="none", - beam_shape="off") + def test_picking_before_the_region_has_a_radius_errors(self, window): + """There is no "off" shape any more, so the only way to have no usable + region is the moment before the detector size has filled the radii in — + which is what ``BeamRegion.active`` still guards.""" + session, plot, _tree, wiz = _opened(window, center_mode="none") + wiz.params["beam_r"] = 0.0 dpca.dpc_pick_center(session, plot, {}) assert any("beam region" in e for e in _errors(window["messages"])) @@ -338,69 +342,167 @@ def test_vacuum_before_a_dataset_is_picked_is_not_an_error(self, window): assert wiz.result is not None assert not _errors(window["messages"]) - def test_the_beam_region_widget_matches_the_shape(self, window): + def test_the_beam_region_selector_matches_the_shape(self, window): """Circle and ring are different anyplotlib widget TYPES, so switching - rebuilds rather than mutates — and the widget on screen must be the one - the mask is computed from.""" + rebuilds the SELECTOR rather than mutating it — and the widget on screen + must be the one the mask is computed from. + + The region is a real ``BaseSelector`` (the same one a virtual image puts + on this plot), which is what gives it the navigator's latest-wins + dispatch and settle re-fire instead of a hand-rolled debounce. + """ + from spyde.drawing.selectors import AnnularSelector, CircleSelector + session, plot, _tree, wiz = _opened(window, beam_shape="circle") - assert type(wiz._beam_widget).__name__ == "CircleWidget" + assert isinstance(wiz._beam_selector, CircleSelector) + assert type(wiz._beam_selector.roi).__name__ == "CircleWidget" assert wiz.region().shape == "circle" and wiz.region().r > 0 dpca.dpc_set_beam(session, plot, {"beam_shape": "ring"}) - assert _wait(lambda: type(wiz._beam_widget).__name__ == "AnnularWidget") + assert _wait(lambda: isinstance(wiz._beam_selector, AnnularSelector)) + assert type(wiz._beam_selector.roi).__name__ == "AnnularWidget" assert wiz.region().shape == "ring" assert 0 < wiz.region().r_inner < wiz.region().r - dpca.dpc_set_beam(session, plot, {"beam_shape": "off"}) - assert wiz._beam_widget is None and not wiz.region().active + def test_writing_the_region_also_refreshes_the_panel(self, window): + """A geometry write from Python must leave the FIGURE's own state + agreeing with the widget, not just the widget. + + ``Widget.set`` reaches the figure as a targeted message and deliberately + never rewrites the panel state — and the renderer retains that state and + re-applies it, so whatever the panel still holds is what the figure falls + back to. What it held was the geometry the widget was BORN with + (``image_width * 0.1``, a fifth of the radius the region opens at), and + the circle collapsed to it the moment the pointer entered the pattern. + Dragging the collapsed circle then measured a region a few pixels wide. + """ + _session, _plot, _tree, wiz = _opened(window, beam_shape="circle") + widget = wiz._beam_selector.roi + plot2d = widget._plot + pushes = [] + real_push = plot2d._push + + def counted_push(): + pushes.append(1) + real_push() + + plot2d._push = counted_push + try: + wiz._write_region_to_widget(wiz.region()) + finally: + del plot2d._push + assert pushes, "the panel kept the widget's birth geometry" + drawn = plot2d.to_state_dict()["overlay_widgets"] + assert [w["r"] for w in drawn] == [wiz.region().r] + + def test_the_region_is_always_on_the_pattern(self, window): + """There is no "off". The region IS what the centre of mass is taken + over, so switching it off only meant taking the whole frame with no + handle to grab — a control whose useful setting was "not that one".""" + session, _plot, _tree, wiz = _opened(window) + assert wiz._beam_selector is not None + assert wiz.region().active, "the region opened without a usable radius" + assert "off" not in dpca._dpc.BEAM_SHAPES + assert dpca.DEFAULTS["beam_shape"] in dpca._dpc.BEAM_SHAPES def test_radii_are_filled_in_from_the_detector_size(self, window): """They cannot be declared in DEFAULTS — a sensible radius is a fraction - of a detector whose size is unknown until a dataset is open.""" - session, plot, _tree, wiz = _opened(window, beam_shape="off") - assert wiz.params["beam_r"] == 0.0 - dpca.dpc_set_beam(session, plot, {"beam_shape": "circle"}) + of a detector whose size is unknown until a dataset is open. Opening the + wizard is therefore what fills them in, since the region is always on.""" + session, plot, _tree, wiz = _opened(window) sy, sx = wiz._sig_shape() - assert wiz.params["beam_r"] == pytest.approx(0.25 * min(sy, sx)) + # The INSCRIBED circle: the region is always on, so its default is what + # an unattended scan is measured with, and a smaller one clips the beam + # and under-reads the shift. See dpc.default_beam_region for the numbers. + assert wiz.params["beam_r"] == pytest.approx(0.5 * min(sy, sx)) assert (wiz.params["beam_cx"], wiz.params["beam_cy"]) == (sx / 2, sy / 2) - def test_a_drag_frame_costs_nothing_and_the_release_pays(self, window): - """Every cost waits for ``pointer_up``. + def test_a_moved_region_re_measures_through_the_selector(self, window): + """The map tracks the region as it moves, the way a virtual image tracks + its detector ROI — and for the same reason: the region IS a + ``BaseSelector``, so every move goes through the navigator's serial + latest-wins dispatcher and cancels the compute it replaces. - Two of them. Re-measuring the scan is the obvious one. The other is the - brightness readout, which reads a frame — a dask compute on a lazy scan - — and firing one per pointer frame queues work faster than it drains, - so the caret's own radius keeps climbing after the pointer has stopped. - A drag frame must therefore do no reading at all, only echo geometry. + Waiting for ``pointer_up`` left the map frozen for the whole gesture + (measured in the real app: zero repaints across a 6.4 s drag). """ session, plot, _tree, wiz = _opened(window, beam_shape="circle") - measures = {"n": 0} - reads = {"n": 0} + passes: list = [] + wiz.measure = lambda **kwargs: passes.append(wiz.region().r) + + # Driven by moving the WIDGET, not by calling the hook: what is under + # test is that the selector carries a move all the way to a re-measure + # on its own, which is the whole point of it being a BaseSelector. + for r in (8.0, 9.0, 10.0, 11.0): # a drag, frame by frame + before = len(passes) + wiz._beam_selector.roi.set(r=r) + assert _wait(lambda: len(passes) > before, timeout=10.0), \ + f"moving the region to r={r} never re-measured" + + assert passes[-1] == pytest.approx(11.0), \ + "the last pass measured a region the widget had already moved off" + assert wiz.params["beam_r"] == pytest.approx(11.0), \ + "the widget geometry must track the pointer every frame" + + def test_a_superseded_pass_is_cancelled_not_left_to_finish(self, window): + """Each move cancels the pass it replaces. A beam-shift pass reads the + whole scan, so one nobody is waiting for costs the cluster the entire + dataset — the same contract ``virtual_image`` keeps.""" + session, plot, _tree, wiz = _opened(window, beam_shape="circle") + first = [False] + wiz._track_measure(first) + assert wiz._measure_stop is first + + wiz._beam_selector.roi.set(r=9.0) + assert _wait(lambda: first[0], timeout=10.0), \ + "moving the region left the previous pass running" + assert wiz._measure_stop is not first, \ + "the new pass did not take the slot from the one it superseded" + + + def test_moving_the_region_does_not_read_a_frame_for_the_brightness(self, window): + """The brightness readout reads a FRAME — a dask compute on a lazy scan + — so one per pointer frame queues work faster than it drains, and the + caret's own radius keeps climbing after the pointer stops. Re-measuring + has no such problem (it dispatches and returns, and a superseded pass is + cancelled), which is why the two are not on the same cadence: the + readout refreshes when a pass LANDS. + """ + import threading + import spyde.actions.dpc as dpc_mod - real_measure = dpc_mod.measure_beam_shifts + reads: list = [] real_brightness = dpc_mod.region_brightness - dpc_mod.measure_beam_shifts = lambda *a, **k: ( - measures.__setitem__("n", measures["n"] + 1) or real_measure(*a, **k)) dpc_mod.region_brightness = lambda *a, **k: ( - reads.__setitem__("n", reads["n"] + 1) or real_brightness(*a, **k)) + reads.append(threading.current_thread()) or real_brightness(*a, **k)) try: - for r in (8.0, 9.0, 10.0, 11.0): # a drag, frame by frame - wiz._beam_widget.set(r=r) - wiz._on_region_drag(_drag_frame()) - assert measures["n"] == 0, "a drag frame re-measured the whole scan" - assert reads["n"] == 0, \ - "a drag frame read a frame for the brightness readout" - assert wiz._settle_timer is None, "a drag frame armed the re-measure" - assert wiz.params["beam_r"] == pytest.approx(11.0), \ - "the drag must still track the widget, it just must not read" - - wiz._on_region_drag(_release()) - assert reads["n"] == 1, "the release did not refresh the brightness" - assert _wait(lambda: measures["n"] >= 1, timeout=10.0), \ - "the release never fired the re-measure" - assert measures["n"] == 1, "the settle should coalesce to ONE measure" + session, plot, _tree, wiz = _opened(window, beam_shape="circle") + wiz.measure = lambda **kwargs: None # not what this test is about + # The OPENING pass ends with a brightness read of ITS own, on a + # worker — and `_opened` returns from the line before it is + # submitted (`window_id` is set by the `refresh` just above it). So + # wait for that read to land and go quiet; clearing straight away + # leaves it to arrive inside the window measured below, where it is + # indistinguishable from a drag having read a frame. + assert _wait(lambda: reads, timeout=30.0), \ + "the opening pass never read the brightness" + settled = -1 + while settled != len(reads): + settled = len(reads) + time.sleep(0.2) + reads.clear() + for r in (8.0, 9.0, 10.0, 11.0): + wiz._beam_selector.roi.set(r=r) + wiz._on_region_moved() + assert reads == [], \ + "moving the region read a frame for the brightness readout" + + wiz.emit_region(with_brightness=True) # what a landed pass does + assert _wait(lambda: len(reads) >= 1, timeout=10.0), \ + "a landed pass never refreshed the brightness" + assert reads[0] is not threading.current_thread(), \ + "the brightness probe ran on the caller's thread, stalling the drag" finally: - dpc_mod.measure_beam_shifts = real_measure dpc_mod.region_brightness = real_brightness def test_a_measure_abandoned_by_close_does_not_shout(self, window): @@ -429,18 +531,22 @@ def test_a_superseded_measure_does_not_shout(self, window): wiz._measure_failed(stale, RuntimeError("boom")) assert len(_errors(window["messages"])) == before - def test_the_drag_debounce_cannot_fire_after_teardown(self, window): + def test_the_settle_re_fire_cannot_fire_after_teardown(self, window): """A timer that survives close would re-measure a torn-down wizard on a - worker thread.""" + worker thread. The timer belongs to the SELECTOR now, so closing the + wizard has to close the selector — which is the one thing that could + quietly stop happening when the region stopped being a raw widget.""" session, plot, _tree, wiz = _opened(window, beam_shape="circle") - wiz._on_region_drag(_release()) - assert wiz._settle_timer is not None + selector = wiz._beam_selector + selector.update_data() # arms the settle re-fire + assert _wait(lambda: selector._settle_timer is not None, timeout=5.0) dpca.dpc_close(session, plot, {}) - assert wiz._settle_timer is None and wiz._closed + assert selector._settle_timer is None and wiz._closed + assert wiz._beam_selector is None def test_the_region_is_echoed_back_for_the_caret(self, window): session, plot, _tree, wiz = _opened(window, beam_shape="circle") - wiz._on_region_drag() + wiz._on_region_moved() msgs = _of_type(window["messages"], "dpc_region") assert msgs, "no dpc_region echo reached the caret" last = msgs[-1] @@ -537,9 +643,13 @@ def test_a_lazy_scan_fills_in_progressively(self, window): each lands — the difference between a filling field and a spinner on a scan that takes minutes. - Asserted on the PROGRESS stream and on partial state, not on timing: a - single final repaint would emit one progress message and never expose a - half-NaN field. + Asserted on the PROGRESS stream, which reports what each landing chunk + triggered. NOT on how much of the field was finite when a repaint ran: + the repaints are marshalled to the event loop and the pass now finishes + in milliseconds, so every one of them can legitimately see a complete + field and still have been driven by a separate chunk. That is the + compute being fast, not the stream having collapsed — measuring it that + way made this test fail on a 37x speedup. """ import dask.array as da import hyperspy.api as hs @@ -565,34 +675,60 @@ def spy(self): return real_refresh(self) total_positions = nav * nav + chunks = (nav // chunk) ** 2 + + def streamed(): + """The progress a LANDING CHUNK reported, one message each.""" + return [m for m in window["messages"] + if isinstance(m, dict) and m.get("type") == "progress" + and "DPC" in str(m.get("label", "")) + and m.get("total") == chunks] + + def finished(): + """`_finish` is the only thing that emits a bare ``"DPC"`` + progress, and it runs only once the LAST chunk is in.""" + return any(isinstance(m, dict) and m.get("type") == "progress" + and m.get("label") == "DPC" and m.get("total") == 1 + for m in window["messages"]) + dpca.DpcWizard.refresh = spy try: dpca.dpc_open(session, plot, {}) _wait(lambda: getattr(plot.signal_tree, "_dpc_wizard", None) is not None) - # Wait on the recorded PAINT, not on `wiz.shifts`: `_finish` assigns - # the shifts and only THEN calls `refresh`, so a wait on the array - # can return between the two — leaving the last entry in - # `seen_partial` a partial paint, and restoring the spy in `finally` - # before the full one was ever recorded. + # Wait for the PASS TO FINISH — not for the field to be complete. + # The opening pass seeds the whole field from the scan corners + # before a single chunk has landed, so "every position is finite" + # is already true at the FIRST repaint: waiting on it returned + # after one chunk of four and then read a progress stream three + # messages short of what the pass went on to emit. + assert _wait(finished, timeout=60.0), \ + f"the streamed pass never completed: {streamed()}" + # Every chunk but the last reports progress; the last one IS the + # finish. Both emits happen on their own chunk's callback thread, + # so the finish can be recorded a moment before the partial that + # precedes it — hence a wait rather than a bare read. + _wait(lambda: len(streamed()) >= chunks - 1, timeout=5.0) + # Only now is the last recorded paint the completed field. assert _wait(lambda: seen_partial and seen_partial[-1] == total_positions, - timeout=60.0), \ - f"the streamed pass never completed: {seen_partial}" + timeout=10.0), \ + f"the finished pass never repainted the whole field: {seen_partial}" finally: dpca.DpcWizard.refresh = real_refresh assert seen_partial, "the map was never repainted" - # The whole claim: at least one repaint happened while the field was - # still incomplete. - assert any(0 < n < total_positions for n in seen_partial), \ - f"no partial repaint — the map only appeared at the end: {seen_partial}" - - progress = [m for m in window["messages"] - if isinstance(m, dict) and m.get("type") == "progress" - and "DPC" in str(m.get("label", ""))] - assert len(progress) >= 2, f"progress was not streamed: {progress}" - assert progress[0]["total"] == (nav // chunk) ** 2, \ + assert seen_partial[-1] == total_positions, \ + f"the streamed pass left holes in the field: {seen_partial}" + + # The whole claim: the map was repainted MORE THAN ONCE, once per nav + # chunk that landed, rather than a single paint at the end. + landed = streamed() + assert len(landed) >= 2, \ + f"the pass did not stream — one paint at the end: {landed}" + assert any(0 < m["done"] < chunks for m in landed), \ + f"no chunk reported partial progress: {landed}" + assert landed[0]["total"] == chunks, \ "progress should count NAV CHUNKS, matching the storage layout" def test_an_eager_scan_does_not_pretend_to_stream(self, window): @@ -831,11 +967,44 @@ def test_the_worker_never_gets_the_trees_own_signal(self, window): "the worker was handed the tree's live signal — two passes would race" assert handed_data is live.data, "the view must share the data buffer" - def test_overlapping_passes_get_separate_objects(self, window): - _tree, seen = self._measured_signals(window, opens=3) - assert len(seen) >= 2 - assert len({id(signal) for signal, _data in seen}) == len(seen), \ + def test_every_pass_gets_its_own_object_and_they_cannot_overlap(self, window): + """Two guarantees, and the second is what makes the first sufficient. + + Each pass is handed its own view, so two of them cannot read each + other's state. And they are set up on a SINGLE-thread lane, so two + set-ups never run at once — which is the stronger property, because + ``private_view`` is itself the operation that is unsafe to overlap + (hyperspy parks a length-1 placeholder on ``.data`` while it copies). + + Passes are run sequentially here rather than raced: a superseded pass is + now cancelled before the lane ever picks it up, so open/close/open no + longer produces two calls to compare. + """ + session, plot, _tree, wiz = _opened(window) + seen = [] + real = dpca._dpc.measure_beam_shifts + + def spy(signal, **kw): + seen.append(signal) + return real(signal, **kw) + + dpca._dpc.measure_beam_shifts = spy + try: + for _ in range(3): + before = len(seen) + wiz.measure() + assert _wait(lambda: len(seen) > before, timeout=30.0), \ + "a measure never reached the worker" + finally: + dpca._dpc.measure_beam_shifts = real + + live = dpca._current_signal(_signal_plot(session)) + assert all(s is not live for s in seen), \ + "a pass was handed the tree's live signal — two would race" + assert len({id(s) for s in seen}) == len(seen), \ "two measures shared one signal object" + assert wiz._pass_lane()._max_workers == 1, \ + "the pass lane must be serial — private_view cannot overlap itself" @pytest.mark.usefixtures("_capture_module_emit") @@ -867,6 +1036,43 @@ def test_open_close_open_leaves_exactly_one_wizard(self, window): f"{len(opened - closed)} DPC windows left open " f"(opened {sorted(opened)}, closed {sorted(closed)})") + def test_a_close_mid_pass_leaves_no_window_behind(self, window): + """A pass that has already cleared its ``_closed`` check must not go on + to open a window for a wizard that is gone by the time it gets there. + + ``_finish`` runs wherever the pass landed and ``dpc_close`` on the + caller's thread, so the two interleave. ``remove`` closes the window it + can SEE — and it sees none, because the pass opens it a moment later. + Nothing owns that window afterwards and nothing ever closes it. This is + the StrictMode open/close/open sequence with the timing pinned instead + of raced, which is what made it a failure on some machines only. + """ + session, plot, tree = _dataset(window) + entered, go = threading.Event(), threading.Event() + real_derive = dpca.DpcWizard.derive + + def gated_derive(self): + entered.set() + go.wait(30.0) + return real_derive(self) + + dpca.DpcWizard.derive = gated_derive + try: + dpca.dpc_open(session, plot, {}) + assert entered.wait(30.0), "the pass never reached the map" + dpca.dpc_close(session, plot, {}) + finally: + go.set() + dpca.DpcWizard.derive = real_derive + time.sleep(0.5) # let the released pass finish whatever it does + + opened = {m["window_id"] for m in _of_type(window["messages"], "figure") + if str(m.get("title", "")).startswith("DPC")} + closed = {m["window_id"] for m in _of_type(window["messages"], + "window_closed")} + assert not (opened - closed), \ + f"orphan DPC window(s) {sorted(opened - closed)} — closed already ran" + def test_re_opening_a_live_wizard_does_not_build_a_second(self, window): session, plot, tree, wiz = _opened(window) dpca.dpc_open(session, plot, {"rotation": 15.0}) diff --git a/upcoming_changes/+dpc-beam-region-collapse.bugfix.rst b/upcoming_changes/+dpc-beam-region-collapse.bugfix.rst new file mode 100644 index 00000000..6dfd5399 --- /dev/null +++ b/upcoming_changes/+dpc-beam-region-collapse.bugfix.rst @@ -0,0 +1,5 @@ +The DPC beam region no longer collapses the moment the pointer enters the +diffraction pattern. Its radius was being written to the widget alone, while +the figure's own state kept the radius the widget was created with — a fifth +as large — and reverted to it on the next redraw, so the region that was +actually measured was a few pixels wide and dragging it moved that. diff --git a/upcoming_changes/+dpc-live-region.bugfix.rst b/upcoming_changes/+dpc-live-region.bugfix.rst new file mode 100644 index 00000000..ea923bb0 --- /dev/null +++ b/upcoming_changes/+dpc-live-region.bugfix.rst @@ -0,0 +1,4 @@ +The DPC field map now recomputes as the beam region is dragged, instead of +staying frozen until the pass before it had finished. The beam region is a +real selector, so a superseded measurement is cancelled rather than left to +run, and the centre of mass is ~37x faster. diff --git a/upcoming_changes/+dpc-region-always-on.api_change.rst b/upcoming_changes/+dpc-region-always-on.api_change.rst new file mode 100644 index 00000000..c0ed3355 --- /dev/null +++ b/upcoming_changes/+dpc-region-always-on.api_change.rst @@ -0,0 +1,4 @@ +The DPC beam region no longer has an "off" setting — it is what the centre of +mass is taken over, so it is always on the pattern. Its default radius is now +half the shorter detector axis rather than a quarter: with the region always +on, a smaller default clipped the beam and under-read every field.