Direct Electron .csb — read the event stream, scrub it, and re-cut it To Frames - #90
Merged
Conversation
… Frames A CSB file is not a frame stack — it is a compressed sparse block stream of detected electron events at the camera's raw cadence (390 us on the test movie), and a single raw frame of it is mostly empty pixels. An image only exists once you choose an exposure and integrate, so "opening" one means choosing one, and the reader's job is to make that choice cheap to change and cheap to scrub. Opening gives a stack of integrated time planes, ONE PLANE PER DASK BLOCK, so scrubbing integrates only the window it is parked on — the property the navigator rests on, and the one the sibling rosettasciio/mrc patch documents a 40x scrub regression from losing. "To Frames" then re-cuts the same stream at whatever frame rate you ask for, as a new lazy node in the signal tree; from there it is an ordinary insitu movie and Play, virtual imaging, the movie editor and the rest work on it with no idea where it came from. Written as a DROP-IN rsciio plugin (spyde/external/rsciio_csb/: __init__ re-exporting file_reader, _api.py, specifications.yaml, importing nothing from SpyDE) so moving it into RosettaSciIO is a directory copy plus deleting the runtime registration. _core/_sparse are vendored verbatim from de-csb. Three things that had to be got right, all measured on the real 8192x8192, 400-frame, 125.8M-event movie: * `dask.delayed(pure=True)` tokenizes its arguments, and passing the dataset meant dask hashed the 252 MB memory-mapped payload once per plane: 70 SECONDS to build a graph that reads nothing. The graph now carries the file PATH and resolves a cached dataset per worker — which is also what makes it picklable and gives the accumulator a single owner. Open is now 35 ms warm. * The navigator is FREE from the block table (counts per frame, zero payload reads, 1.7 ms) and exactly equals what integrating yields. It travels as original_metadata — the signal dict's `attributes` key does NOT reach the signal object, so a navigator sent that way silently vanishes and SpyDE integrates the whole movie to draw a thumbnail anyway. * The accumulator is not concurrency-safe (reset+add_frames mutate one shared buffer), so integrations take a lock. Per-thread accumulators would cost 268 MB each at 8192" to win back parallelism the GPU serialises regardless. Also: `requires_original_metadata`, a general format gate mirroring requires_vectors, because "re-cut the exposure" is a question you can only ask of an event stream and signal_type cannot express that (a CSB movie is an ordinary insitu signal and must stay one). And _load_file_thread now ensures spyde.external's patches have landed before hs.load — it did not, so a load that beat the startup prewarm saw an unpatched rosettasciio: no CSB reader, and silently the slow MRC path.
Two things that made the reader look worse than it is. **The GPU was never used.** _core's GPU lane is a CuPy RawModule kernel, and SpyDE does not install CuPy — its GPU stack is torch, pinned with CUDA. So on a SpyDE machine gpu_available() was False for want of a package nobody has, and every integration fell to the CPU. _torch.py adds the missing lane without touching the vendored _core/_sparse (which stay diffable against upstream de-csb): a CSB event word IS its intra-block raster index, so the whole accumulate is a scatter-add of ones at block_id * tile_size + word — no custom kernel, just index_add_. Measured on the real 8192², 125.8M-event movie against numba, bit-identical both ways: one plane (what the slider integrates) 487 ms -> 156 ms 3.1x whole movie (125.8M events) 544 ms -> 624 ms 0.9x The per-plane win is the one that matters; the whole-movie case is transfer-bound, not arithmetic-bound, which is also why the words go across PCIe as int32 and are widened to the int64 index_add_ wants on the device (sending int64 cost another 50 ms/plane). numba stays available for whole-file sums via backend="cpu-numba". **Open could not select a .csb.** SUPPORTED_EXTS had it but the Electron file dialogs carry their own extension filters, in two places, and neither listed it — so the format was registered, loadable by path, and unreachable from the menu. Both updated, plus a dedicated "DE CSB (.csb)" filter row.
Drove the real 8192², 125.8M-event movie through the app (csb_movie.spec.ts):
it opens as an insitu movie with the free block-table navigator, the time
slider integrates a fresh window per position, and To Frames adds a second
dataset to the tree ("16 planes at 10 ms (100 fps)").
Profiling a plane says the events were never the problem:
memmap read + host cast + H2D + repeat_interleave + idx + index_add_ ~10 ms
image() — the D2H of the 8192² result ~212 ms
2.5M events accumulate at ~250 M ev/s; shipping 268 MB back over PCIe is 95%
of the wall clock. So the copy now goes through a reusable pinned buffer
(212 -> 168 ms; a pageable D2H bounces through a driver staging buffer), and
the docstring points at preview() for anything that only needs looking at —
at bin 4 that is 1/16th of the bytes and 36 ms/plane against 179.
Still bit-identical to numba, and 3.6x on the per-plane window the slider uses.
The remaining full-res cost is the device-side untile copy plus the transfer
itself; a 268 MB plane has a hard floor here and the way past it is to stop
moving whole planes (a TileBackend over the accumulator), not micro-tuning.
…s/step
de-csb's README already answered this, under "Readout is a separate cost —
don't count it against real-time": pulling the full 8192² image back caps you
at ~11 Hz while the 4x-binned preview runs at ~84 Hz, so display from the
binned one and call image() only when something is measured. My own profile
agreed independently — accumulating 2.5M events is ~10 ms, the readback ~200.
So a CuPy kernel is the wrong lever. The repo's table has it at 2,230 M ev/s
against this torch path's ~250 M, which sounds decisive until you notice it is
9x on the 10 ms, i.e. ~5% of a plane. Left alone; `auto` already prefers
_core's CuPy kernel if anyone installs it.
What actually moved the needle, on the real 8192² movie:
bin on the DEVICE _sparse's _sum_frames pulls the full image and bins on
the host, so asking for bin=4 cost MORE than bin=1 — the
whole 268 MB transfer plus a host bin. _TorchSparseCSB
overrides it to use preview(), which bins before the
copy: 1/16th of the bytes.
auto bin default keeps a plane's longest edge at 2048, so an
8192² movie scrubs 2048² planes. bin=1 still available.
plane cache 512 MB LRU keyed by the same tuple the graph carries. A
drag walks back and forth over the same planes and each
repeat was paying the readback again.
first visit 330 -> 57 ms revisit 177 -> 22 ms
24-step drag 354 -> 37 ms/step (~27 fps)
Bit-identical to numba at bin 1, 2 and 4, counts conserved. Binning is a SUM,
and on data this sparse (~1.9 counts/px for the ENTIRE movie) it is also what
makes a plane visible at all — the bin=1 view is near-black speckle, the
binned one shows the particles.
… plane 0
The navigator I hand to _add_signal was a bare Signal1D of the block-table
counts, with no axis calibration. But a 1-D selector turns the widget position
into an index with (x - offset) / scale read from THE NAVIGATOR PLOT'S OWN
signal axis (selector1d._signal_axis) — so the widget's x in seconds was being
divided by a scale of 1.0, and every position on a 0.156 s movie resolved to
index 0. Inspected directly:
SIGNAL nav axis : time, 50, scale 0.00312, units s
NAVIGATOR sig ax: <undefined>, 50, scale 1.0, <undefined>
The navigator now carries the source signal's navigation calibration, in both
places one is built (the reader path and To Frames).
The spec that was supposed to cover this was worthless three times over, which
is worth recording because each failure looked green:
* counting bright pixels page-wide never changes with the plane, and the
navigator's own canvas keeps it non-zero regardless;
* reading canvas pixels with getImageData returns all zeros — the image is
on a WebGPU canvas (the trap the vectors-embed work hit). It "failed"
without the fix for the wrong reason: it read 0:0 either way;
* page.mouse.click on the navigator moves the CURSOR, not the widget — the
readout showed x:0.0297 while the selector marker sat at 0.
It now hashes a screenshot of the signal window (composited truth) and moves
via test_nav_drag, and it does prove the per-plane read/paint path tracks.
HONEST LIMIT: test_nav_drag sets indices directly, so it does NOT exercise the
selector's axis conversion and would not have caught this bug. Driving the
VLine widget from a spec has no hook today (insitu_playback works around the
same gap); the calibration is verified by inspection, not by test.
Integrating by default is right — a raw CSB frame is 390 us and ~0.005 counts/px, so on its own it is very nearly an empty image. But that is exactly why it has to be reachable: "the default is a sum" is only a safe default if you can still see what was summed. `frames_per_plane` was already the knob (it takes precedence over fps and exposure in _resolve_exposure) but it was a hs.load kwarg only, so from the UI there was no way to ask for one. Now a To Frames parameter: 1 gives one plane per raw frame — 400 planes of 390 us here — and 0 keeps the fps/exposure fields in charge. Verified against the real movie: default 50 planes, 3.12 ms, 8 raw frames each 0.603 counts/px (bin 4) raw 400 planes, 0.39 ms, 1 raw frame each 0.075 counts/px The status line now reports the raw-frame count alongside the exposure — "16 planes at 10 ms (100 fps), 32 raw frames each", or "1 raw frame each — no integration", because the time alone does not tell you how much of the stream went into a plane. Also exposed bin as "0 = auto" rather than hiding the auto-binning behind a default of 1 that no longer meant anything.
A point selector reads one navigation position. This lets it read n and sum
them, keeping the single pointer. Distinct from Integrate, which hands you a
draggable span with two edges to place: this is for when the exposure is a
property of the acquisition rather than a region you are choosing — summing n
frames of an in-situ movie for signal, or picking the exposure of a sparse
event stream, without turning the slider into a range.
Generic, not CSB-specific, per review: any 1-D (movie/time) navigator gets it.
A 2-D navigator does not, because "n frames" has no unambiguous direction
there and Integrate already covers it — the backend simply omits sum_frames
and the dock hides the control.
Downstream needs no new path: emitting n index rows is exactly what a region
selector emits, so the existing region-integration machinery (array_cache,
region_sum's threaded bands and incremental +-1) handles it unchanged.
The ladder is powers of two, labelled with the rate each produces when the
axis is time ("8 frames — 40 fps"). Frame counts rather than round frame
rates because a rate cannot generally divide the cadence — asking for 60 fps
on a 2564 fps camera silently rounds to 43, so the number picked is not the
number you get.
Two things that would have been silent bugs:
* the live selector is IntegratingSelector1D, a COMPOSITE. Its __getattr__
delegates reads only, so `composite.sum_frames = 8` bound on the wrapper
while the inner line selector kept 1 — the attribute read back as 8 and
nothing summed. It is an explicit property now.
* the window SLIDES at the ends rather than being clipped. A clipped window
folds onto the edge index and sums the same frame several times, quietly
making the first and last positions brighter than the rest.
The picker now hangs off the Point button as a bare caret, and the button carries the width it is reading as a subtle badge — "✛ Point 8f" — so you can see what the pointer is summing without opening anything. Dropdown gained a `triggerText` override for this: the menu keeps the full "8 frames — 40 fps" labels while the trigger shows nothing, because repeating the selection next to a control that already displays it is just noise. Also fixes a swallowed exception in set_selector_sum. It called update_data(force=True), which that method does not accept; the raise happened BEFORE the confirming emit, so the width applied to the selector and the dock never heard about it. The pointer really was summing 8 frames while the button still said 1 — and the test passed anyway, because it only checked that the image changed. It now asserts the button reports the width back. NOT flipping the CSB load default to raw cadence (asked for, conditional on performance — it fails that condition). Summing n positions means n accumulator passes and n readbacks, where pre-integrating them is one of each, and the readback is where the time goes: one pre-integrated 8-frame plane (today) 37.6 ms eight raw planes summed, cold 340.8 ms 9.1x sliding one step, 7 of 8 cached 108.6 ms 2.9x Identical results, so this is purely cost. Raw cadence stays reachable through To Frames -> Raw frames per plane = 1, which pays it only when asked for.
It was a third control sitting between Point and Integrate. Now Point is a split control — "✛ Point 8f ▾" — with the caret at its right edge. Buttons cannot nest, so this is a styled wrapper carrying the active/inactive look with two transparent children inside it. Dropdown gained `bare` (drop the trigger's own border and background so it disappears into a host control) and `caretColor` (stay legible on whatever that host's background is). Two things that cost a round trip each, both worth remembering: * `overflow: hidden` on the wrapper CLIPPED the menu. The options were in the DOM and permanently invisible — Playwright's "locator resolved to <button …>8 frames — 40 fps</button>, 57 x waiting for element to be visible" is exactly that shape, and it is not a timing problem however much it reads like one. * a <button> does not inherit the parent's colour from the UA stylesheet, and the `font` shorthand in an inline style wiped the label outright. Both are set explicitly now rather than inherited.
…promised csb_format.py's own docstring says "test_csb_reader.py asserts the two agree" about _SPEC and specifications.yaml. That file did not exist, and nothing else covered the CSB path in CI either: csb_movie.spec.ts drives the real thing but skips itself when the multi-gigabyte stream is absent, which on a runner is always. The event decoder genuinely needs that stream. Everything above it does not: a valid CSB carrying zero events is a 108-byte header plus a zero-filled block table, with data_offset == lengths_offset so the payload region is empty and the reader's own "table sums to N, payload holds M" check is satisfied. So this covers the failures that would be silent rather than loud — the plugin not registering (.csb quietly refuses to open), .csb missing from SUPPORTED_EXTS (the Open dialog will not offer it, which is a bug this branch already had to fix), _SPEC drifting from the yaml that becomes authoritative upstream — plus the header offsets, the block grid and its padding for partial edge blocks, and the four ways a file is rejected. The zero-dimension cases needed the fixture padded past 110 bytes: an empty table leaves a 108-byte file, which trips the length guard before the validation under test, so they would have passed for the wrong reason.
A .csb loads as integrated planes — 8 raw frames each — and that stays the default, because one raw 390us frame of an 8192^2 detector carries ~314k events across 67M pixels (0.5% occupancy) and is mostly noise. But the default also hides what the format is actually streaming, so the width ladder now goes one step BELOW a plane: "1 raw frame — 2.6 kfps", with the button reading `raw` so you can see which you are looking at. A raw frame is not a slice of the loaded signal — the signal is a stack of PLANES, a frame is a different integration. original_metadata.csb's plane_frame_bounds maps a plane to its [f0, f1) raw range, so the frame is integrate_plane(path, backend, f0, f0+1, bin, dtype): the very call the plane stack is built from, so same binning, same shape, same cache. It installs by swapping the child's entry in selector.children — the seam the vectors tree already uses to COMPUTE a frame rather than slice one — and restores whatever was there before, so a tree with its own render hook gets its own back. Nothing in the navigator read path changes. Costs about what a plane costs (~31ms vs ~39ms measured): the readback is ~27ms and does not depend on the exposure, which is exactly why this is a viewing mode and not a load option — it is reading the WHOLE movie at raw cadence that multiplies out (12.3s vs 1.9s). The dock only offers it when the backend sends raw_per_plane, which it reads off the TREE ROOT: selector.current_plot is the navigator (whose signal is the free plane-counts overview, no source metadata) and `child` has no signal yet at that point — create_plot_states runs after the emit. Verified against the real 8192^2 stream: the raw frame is the expected sparse speckle where the plane shows structure, the badge reads `raw`, and leaving raw mode restores the integrated plane. All 5 CSB specs pass.
PR #90 targets feat/0.3.0 and had exactly ONE check — the docs Build. The whole test workflow triggers on `pull_request: branches: [main]`, so a PR into the branch that collects an entire release ran no Python matrix, no extras and no e2e. It reported MERGEABLE / CLEAN because nothing failed, because nothing ran. That is not specific to #90: #82 through #88 each landed a 0.3.0 wave that way, and the matrix first saw any of them when 0.3.0 itself went at main (#89) — which is where the timeouts and the four latent races surfaced all at once. `feat/[0-9]*` matches the release integration branches (feat/0.3.0) and not ordinary topic branches, so this costs a matrix per PR into a release line and nothing elsewhere.
…settled Surfaced the moment PRs into feat/0.3.0 started running the matrix: fit_drag_latency failed with settled = -1 — the model never reached its target in 12s, not merely over the 700 ms budget. `target` was captured after a flat 6s wait. If the first drag had not finished settling, that is a moving goalpost and the 2%-of-swing window can never be hit, so the loop runs out and reports "never caught up" for a reason that has nothing to do with latency. Sample until it stops moving instead. The 700 ms budget is untouched — this makes the reference real rather than relaxing the bar it is measured against. Locally: 29 moves over 505 ms, model settled 119 ms after the last.
7 tasks
…ity (#91) Closes the gap #91 describes. This was more tractable than I first said: `_core.py` documents the format, so a fixture with real events is the zero-event one plus a payload — header, then one uint16 per event giving its raster index inside its own block, then a per-block count footer. Because the mapping is two lines of arithmetic, every expectation is computed in plain numpy, independently of the reader — no golden blobs. `_origin` is deliberately a SECOND implementation of block_origin rather than a call to it; a shared helper would agree with itself regardless of what either did. Covered: events land where the arithmetic says, two events on one pixel add, the reference decoder (frame_events) agrees with the accumulator, csb_order 0 vs 1 on a 4x3 grid (a square grid hides it), a partial edge block crops rather than wraps, frame ranges include and exclude the right frames and are additive, bad ranges are refused, bin=n equals box-summing the unbinned, plane_counts matches the decoded frames, and the docstring's own claim that every CPU path is bit-identical to the GPU one. Verified they bite, by mutation. block_origin swapped -> caught, but only by the reference-path test, because the accumulator computes origins independently; so the fast path was mutated separately (integration range off by one) -> caught by five. Both restored. Still out of scope here: To Frames end to end, which needs the session, and throughput, which belongs in benchmark_*.py.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reads Direct Electron
.csbcentroid-streaming files: a sparse eventstream rather than a dense frame stack, so there is no such thing as
"the frame" until you choose a window to integrate over. Opening one gives
a scrubbable in-situ movie whose time slider integrates a fresh window per
position, and To Frames re-cuts the stream at a chosen exposure into an
ordinary lazy dataset that behaves like any other node in the tree.
Based on
feat/0.3.0, and currentfeat/0.3.0is merged in so thisinherits the CI fixes from #89 — without them these checks would hit the
same 30-minute cancellations.
The reader is not SpyDE code
spyde/external/rsciio_csb/is laid out exactly as a rosettasciio plugin(
__init__re-exportingfile_reader,_api.py,specifications.yaml)and imports nothing from SpyDE, because the intent is for it to move
upstream as
rsciio/csb/— at which point that directory is copied across,rsciio auto-discovers the yaml, and
csb_format.pyplus its entry inexternal/__init__are simply deleted. Registration happens at runtime byappending to
rsciio.IO_PLUGINS, sohs.load("movie.csb", lazy=True)resolves through ordinary extension dispatch and SpyDE's load path is
untouched.
Performance
Scrubbing went 354 → 37 ms/step, from binning on the device and caching
decoded planes (512 MB LRU) rather than re-integrating per move. torch-CUDA
carries the integration where available and is bit-identical to the numba
path. A CuPy kernel was measured and declined: profiling put ~212 ms in
readback against ~10 ms in the events, so the arithmetic was never the
bottleneck — de-csb's own README agrees ("Readout is a separate cost").
Flipping the default load to raw cadence was likewise measured and
declined: 37.6 → 340.8 ms cold and 108.6 ms sliding, which fails the
"as long as it doesn't affect performance" bar. The raw frame stays
reachable through To Frames instead.
Sum frames
A 1-D navigator's Point selector can now read a window of frames rather
than one, chosen from a caret inside the Point button with a subtle
8fbadge so the width is visible without opening the menu. It is general —
any 1-D navigator gets it, not just CSB — and the backend omits
sum_framesentirely for a 2-D navigator, which is what tells the dock notto offer a control whose direction would be ambiguous there.
Testing — read this before merging
test_csb_reader.py(15 tests) is new here, and it is deliberately partial.csb_movie.spec.tsdrives the real 8192², 400-frame, 125.8M-event streamend to end, but skips itself when that file is absent — which on a runner
is always. So before this PR nothing in CI touched any of ~2700 lines.
The event decoder genuinely needs a real stream. Everything above it does
not: a valid CSB carrying zero events is a 108-byte header plus a
zero-filled block table. The new tests therefore cover the failures that
would be silent rather than loud — the plugin not registering (
.csbquietly refuses to open),
.csbmissing fromSUPPORTED_EXTS(the Opendialog will not offer it, a bug this branch already had to fix),
_SPECdrifting from the yaml that becomes authoritative upstream — plus header
offsets, block-grid padding for partial edge blocks, and the four ways a
file is rejected.
Still uncovered in CI: the event decoding, integration and To Frames
maths. Those are exercised only by the e2e on a machine that has the
stream.
Verified
csb_movie.spec.ts— all 4 pass against the real stream, screenshots read.cache and passes warm. The merge does not touch the selector path and
warm runs pass on both sides of it, but I did not run cold against the
pre-merge commit, so I am flagging it as a cold-load fragility rather
than claiming it resolved.