Repository navigation
Lightsheet pyramids: ingest from OME-Zarr or SmartSPIM, chunk materialization, cloud round trip, and napari viewing - #979
Merged
Conversation
…affold) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Code Analyzer flagged the outputs as possibly unset and the args as unused after the arguments block, both correct given the function is a documented scaffold that ends in error(). Initializing img/info before the error clears the return-value warning; %#ok<INUSD> and %#ok<NASGU> silence the unused-arg / unused-assignment findings that are load-bearing for the future implementation. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
Code Analyzer flagged the outputs as possibly unset and every arg as unused after the arguments block, both correct given the function is a documented scaffold that ends in error(). Initializing outputs before the error clears the return-value warning; %#ok<INUSA> on the input names silences the unused-arg findings that name the future signature. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
Code Analyzer flagged it correctly: n_chunks_stored is currently recorded as 0 (materializeChunks path is a scaffold) so nChunks was never read. When materializeChunks lands, n_chunks_stored will be overwritten with what writeChunkFile actually wrote, not prod(chunk_grid). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
Contributor
Both backends this function was to have (source-OME-Zarr and materialized chunk.bin_#) need work the PR does not contain, so its body was a raise-notImplemented that keeps flagging Code Analyzer findings on every push (initialise-outputs / unused-args). Removing the file is cleaner than growing the suppressions until it lands for real. levelTable + chooseLevel remain and are enough for a caller that wants to reason about levels without reading chunks. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
Materializing a level's chunks needs the NDI cloud file-series upload plus a chunk_index side-field on lightsheetZarrLevel; that design is follow-up work. Ship makePyramid without the materializeChunks branch so the package has no notImplemented functions and no Code Analyzer suppressions on placeholder bodies. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
The writeChunkFile scaffold it called is removed in the previous commit as follow-up work. makePyramid now writes exactly what this PR supports: metadata (parent pyramid + one level per multiscales level) with n_chunks_stored = 0. When the file-series upload machinery lands, the option can come back alongside a real writeChunkFile. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
readViewport and writeChunkFile were the scaffold pair removed from this PR; the Python-side companion is at Waltham-Data-Science/NDI-python (`ndi.gui.app.lightsheetZarr`), not VH-Lab/NDI-python. Reflect both in the API surface and the "what still needs building" section. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
Root cause of the TestGeneFromFiles cascade on this PR: every existing document class ships a validator at src/ndi/ndi_common/schema_documents/data/<class>_schema.json, and NDI's schema-registry walk fails hard when a document_class named in database_documents references a validator that does not exist on disk. That fails EVERY session.database_add(...) in the run, which is why tests that touch +gene (nothing to do with our diff) were all "Errored." at the same call site. Add lightsheetZarrPyramid_schema.json and lightsheetZarrLevel_schema.json, mirroring the shape of spatialGeneExpressionPyramid_schema.json and spatialGeneExpressionTiles_schema.json. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
…schemas MATLAB's jsondecode accepts NaN as a JSON extension; every existing NDI schema that has an unbounded matrix dimension uses `[1,NaN]` (imageStack_parameters_schema.json, spatialGeneExpressionTiles_schema.json). The earlier push serialized this via a JSON round-trip which turned NaN into null; the loader then broke schema-registry validation, so every session.database_add(...) in the test run failed at the same call site — the visible cascade on TestGeneFromFiles was a symptom of this, not a per-test error. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
Contributor
Author
|
CI note — the TestGeneFromFiles failures on this PR are not this PR's diff. What is failing on head The failing document class is Why it is not this PR's diff
Expected on a re-run of Standing down on this failure for this PR Generated by Claude Code |
3 tasks
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #979 +/- ##
==========================================
+ Coverage 30.36% 31.62% +1.25%
==========================================
Files 878 898 +20
Lines 44299 46893 +2594
==========================================
+ Hits 13452 14828 +1376
- Misses 30847 32065 +1218 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…el reduction_function, dedupe shared level 0
Splits reduction across the two doc classes correctly. Before this
change lightsheetZarrPyramid carried a single `reduction` field, and
every reduction of a source volume was a separate parent with its own
level ladder — which forced two full copies of level 0 for a store
that ships mean + max pyramids (the common case), and level 0 is
typically ~87% of pyramid storage. Also required rewriting the parent
whenever levels or reductions were added, breaking the point of NDI
document immutability.
Corrected shape:
- `lightsheetZarrPyramid` describes the source volume only. No
reduction, no n_levels, no reductions_available — those are all
ladder-derived and would force a delete-and-rewrite when the ladder
grows.
- `lightsheetZarrLevel` gains `reduction_function` (renamed from
`reduction` for clarity — it names the function, not the outcome).
Values: 'none' for the raw shared level 0, 'mean'/'max'/… for
reduced levels.
- Level 0 is deduped by NGFF `datasets[0].path` — one level doc even
when multiple multiscales entries reference the same array.
Discovery patterns:
- What reductions does this pyramid have?
`{l.reduction_function for l in levels(pyramid)} - {'none'}`
- N levels for reduction R?
`sorted(l for l in levels if l.reduction_function in {'none', R},
key=level)`
- Add a new reduction later? Just write more level docs; no parent
edit.
`makeSourceFile` in the +lightsheet package is rewritten to mirror
main's +gene version: `fileReference` by default (no file bound),
`generic_file` only when `attachFile=true`. OME-Zarr stores are
directory trees, often multi-GB, and the pyramid documents already
carry every byte of them we care about — describing the source is
the right default; attaching a copy would double the storage.
Adds unit tests for the two pure helpers the codecov complaint on
this PR named: `viewCommand` (command builder for the napari
launcher) and `chooseLevel` (target-voxel-size level picker). Both
are called from the manager GUI so their contract matters, and both
are pure enough to test without a session or a display.
Companion updates on the Python side (WDS/NDI-python#296) will
re-vendor the schemas and thread `reduction_function` through the
napari CLI's filter + `--reduction` swap. Coming in a separate push.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
…-wrap
GHAS flagged `{ A{I} }` on line 69. It's the initial-insert path of
the path->reductions map: the value is a 1-cell subarray, and
`perEntryReduction(i)` returns exactly that without the extra
brace unwrap+rewrap.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
The source store's chunk shape is thrown away and each level gets a freshly chosen chunk shape sized to a tileBudgetBytes budget (default 8*2^20 = 8 MB uncompressed). The chooser sizes tiles ISOTROPIC IN WORLD SPACE: each spatial axis gets chunk_i inversely proportional to voxel_i, so a lightsheet volume with 1 um XY and 5 um Z produces pancake tiles rather than voxel cubes. Non-spatial axes (t, c) stay at 1. Across a dyadic pyramid this keeps chunk-in-voxels constant, so chunk_grid halves per axis per level and the tile count drops by 8x per level for a 3D ladder. Coarse levels whose shape has fallen below the ideal tile fall through to a single chunk. 8 MB is sized for a viewer over an ~200 MB/s connection fetching ~4 tiles in parallel per pan gesture: each tile lands in ~20 ms on wire, comfortably below the ~200 ms latency budget with room for HTTP overhead. The number is exposed as tileBudgetBytes on both makePyramid and fromOMEZarr; a caller can also pass explicit `chunks` to override the budget entirely. chooseTileShape landed as its own public function so the Python side can port it directly and the test suite can exercise it without a session; TestChooseTileShape covers the isotropic and anisotropic sweet spots, dtype scaling (uint8/uint16/float32), non-spatial axes, clamping-plus-redistribution at coarse levels, the numpy '<u2' dtype-string form, and a monotonic-decrease-across-a-ladder regression guard. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
Codecov flagged 0% patch coverage on the scaffold. The pure functions were already tested (TestViewCommand, TestChooseLevel, TestChooseTileShape); the session-dependent builders were not. TestMakePyramid drives the pyramid+level writer with synthetic listPyramids structs so the whole schema-level path is exercised without a real store: single-pyramid one-level; the dual mean+max ladder with shared level 0 dedupes to five docs with the right reduction_functions; the auto tile-shape chooser lands each level within budget; explicit `chunks` overrides the budget and is clamped to shape; arity mismatches error out; the parent carries the level-0 frame; and the dependency wiring (subject_id, lightsheetZarrPyramid_id on every level, and every doc actually reaches the database). TestFromOMEZarr uses the NDR fixture that already ships (makeExampleFixture, metadata-only; the same fixture NDR's own tests use, so the ingest path is held to one shape rather than an NDI private synthetic). Covers the end-to-end call, the guards (missing subject, missing store, non-zarr directory, unknown pyramid name), the pyramidNames filter (mean alone -> no dedupe), sourceFileID reuse, and tileBudgetBytes forwarding through to per-level chunks. TestMakeSourceFile mirrors +gene's regression coverage: describe is the default and gives a fileReference the database validates; attach gives a generic_file with generic_file.ext; directory-describe checksums .zattrs (the alternative -- walking a 100 GB tree -- is what the class avoids); attach-a-directory is refused; a missing path errors out; checksum=false leaves the checksum fields empty. TestLevelTable covers the empty-children path (kept as a returned table with the schema columns and the parent frame in UserData), the five-row shared-level-0 ladder (finest-first sort, reductions in the frame), that a bare id string is accepted alongside an ndi.document, the reduction filter (raw + one reduction, three rows), the unknown- parent error, and the exact column names against the schema. TestLightsheetZarrManager exercises the two static methods that do not need a figure: launcherPath (the class default when no preference is set) and deletionPlan (the query wrapper that names every doc database_rm will remove before it does). The GUI callbacks stay uncovered -- headless CI cannot open a figure -- but the manager's non-GUI logic now has a test. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
ismember(charRow, cellstr) treats the char row as ONE string and returns a scalar, so isSpatial collapsed to a single false, chooseTileShape returned [1 1 1] on every input, and 12 of the 13 TestChooseTileShape cases errored on the wrong result. Comparing against 'tc' (a char row on both sides) makes ismember compare element-wise as intended. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
The schema declared both as type "matrix" but their documentation said
"cellstr in MATLAB / list of strings in JSON" and makePyramid passed a
cell array of chars for each. did.database.validate_field_type_and_value
correctly refused a non-numeric matrix, so every path that added a
pyramid document errored with DID:Database:ValidationFieldMatrix -- 16
of the tests I just added blocked at pyramid add before reaching what
they were trying to exercise.
NDI schemas do not have a cellstr type. Simplest honest fix is to make
these single strings and encode the list as comma-separated
("micrometer,micrometer,micrometer", "GFP,mCherry"): both fields are
descriptive metadata, not queryable data, so a single string is enough
and readable. Schema type flipped to "string", default_value to "",
documentation rewritten to name the new encoding, and makePyramid now
strjoins axesUnits with ',' and leaves channel_names empty (we do not
have names in the source yet).
The vendored copy in NDI-python's ndi_common tree is updated to match
so the Python reader validates against the same schema.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
The suite guarded on ndr.test.format.omezarr.makeExampleFixture and filtered when it was absent -- which was the whole CI run, because NDR-matlab's test package is not on the MATLAB path there. Codecov reported fromOMEZarr.m as 0% patch coverage as a result. The fixture is now written inside the test setup: a metadata-only OME-Zarr store (.zattrs + per-level .zarray) with the dual mean+max ladder that shares level 0, matching the shape the ferret-lightsheet lab actually produces. Nothing here writes chunk bytes; fromOMEZarr is metadata-only. NDR is still the reader (ndr.format.omezarr. listPyramids etc.), and the suite still filters cleanly if NDR is absent -- an absent dependency is not a defect in this code. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
New: ndi.test.lightsheet.makeBlobFixture and ingestBlobFixture. The first writes a small OME-Zarr store on disk with real chunk bytes (uncompressed Zarr v2 raw, the shape every zarr reader accepts) and returns the ground-truth volumes it wrote. The second is a one-line wrapper: builds a fresh session, calls the fixture writer, ingests through fromOMEZarr and hands back (session, pyramidDoc, info). The volume is three features at three spatial scales: - a wide Gaussian ball (sigma 40, peak ~50k) at the centre - a medium Gaussian (sigma 15, peak ~30k) offset to a corner - a single-voxel spike of value 60k so the pyramid levels look different at a glance and the mean-vs-max choice is visible (the spike survives max and vanishes under mean). Default shape is 300 x 300 x 300 uint16 (~54 MB in memory), with a dyadic mean + max ladder sharing level 0; chunk shape defaults to 64 per axis but is exposed as a name-value so the same helper covers a 32^3 fast test and a full 300^3 real-fixture run. TestBlobFixture holds it to that contract: .zattrs + per-level .zarray on disk, at least one chunk file per level with the right raw byte size, dual mean + max multiscales with shared level 0, fromOMEZarr ingest producing five deduped level docs and the parent frame matching the source. A separate test builds a 64^3 fixture and verifies max keeps the spike while mean loses it -- the functional-difference test that is the reason this fixture is worth having. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
Code Analyzer flagged the suppression as pointing at a message that no longer fires (MATLAB does not warn on an unused options field). Drop the pragma; the option itself stays, reserved for future noisy fixtures. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
buildBlobVolume put the spike at literal (50,50,50), which at Shape=[32 32 32] silently grew the zeros array to 50x50x50 and then crashed adding it to the 32x32x32 Gaussians. TestBlobFixture used Shape=[32 32 32] for speed, so every one of its six tests errored at setup. Position and width of all three features now scale with Shape: - wide Gaussian sigma = mean(Shape)/8 - medium Gaussian sigma = mean(Shape)/20 - spike position = round(Shape/6), clamped to at least 1 so the fixture is well-defined at any shape, from a 32^3 unit-test size to the 300^3 default. The mean-vs-max regression test now verifies the invariant that actually matters (max at the spike block stays at the spike value, mean is strictly below max there), not a magic 2000 threshold that only holds at one specific Shape. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
Real lightsheet-scale documents (e.g. the 121,441-URL pyramid level that motivated Waltham-Data-Science/NDI-python#320 and ndi-cloud-node#151) sit an order of magnitude past the largest row this probe used to run (N=10000). That regime is where per-URL signing rate goes sub-linear AND the signed-count plateaus for minutes; neither is visible below ~50k. Adding N=50000 and N=100000 gives us the rows that will prove ndi-cloud-node#151's fix has landed and guard against future regressions. Two guards keep the default CI run from swallowing tens of minutes: - **Huge-N gate.** The 50k / 100k rows upload roughly 50k / 100k files each, which currently takes tens of minutes and burns cloud budget on every CI push while the perf issue is still live. ``assumeTrue(NDI_SCALINGPROBE_HUGE=='1')`` skips them by default; set the env var to run them locally when investigating the perf issue. Retire the gate once ndi-cloud-node#151 is fixed and the upload path is fast enough to afford these rows on every push. - **Size-adaptive job timeout.** The old 300 s ceiling was fine for the small rows but would swallow every ready-at-60-90 s finish the big rows will produce once the fix lands: the wait would time out before reporting the actual wall time. Now ``max(300, ceil(N / 100)) s`` -- 300 s for the small rows, 500 s at 50k, 1000 s at 100k -- a generous ceiling costs nothing on the happy path (the wait returns as soon as the job is ready) and lets the row report the real elapsed rather than "timeout". Also expanded the doc comment on ``targetMemberCount`` to name the three size regimes (plumbing / rate-matters / >100k) and the issues each row is regression-guarding. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
Was: the test built a spatial shape via ``pickShapeForTarget(N, [16 16 16])`` that would contain ~N chunks if the fixture wrote at [16 16 16], but its call to ``ndi.fun.doc.lightsheet.fromOMEZarr`` did not forward a ``chunks`` argument. ``makePyramid`` therefore fell through to ``chooseTileShape``'s default ~8 MB per-tile byte budget, which for uint16 lands at roughly [161 161 161] per chunk. Concrete outcome: requested N actual members ------------ -------------- 5 1 100 1 500 1 2000 8 10000 27 50000 64 100000 125 Every row degenerated into a "~125-member series" test, and the scaling axis this class exists to sweep was silently collapsed to a constant. The rows still exercised the bug tracked in #1010 (createSignedURLSetJob rejects a manifest that getFileDetails accepts), but the N label carried no information about scope size -- it only labelled cloud budget spent on a fixture that reproduces the small-N case seven times over. Fix: forward ``'chunks', chunkShape`` to fromOMEZarr so makePyramid uses the chunk shape the fixture wrote (which is the same shape pickShapeForTarget was already sizing the level to). Also add a regression-guard ``warning()`` that fires when the actual member count is <25% of the target for N>=100, so if this ever silently regresses again the run log makes the failure loud rather than legible only by squinting at the [scaleprobe] rows. Companion to Waltham-Data-Science/NDI-python#320 and ndi-cloud-node#151. Once the server-side signing behaviour stabilises, the huge-N rows will actually produce huge-N scopes and the performance-regime evidence they were added to capture becomes meaningful. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
Uncovered by CI on head b0dcce1 (Cloud CI 2): the fixture-count fix made N=10000 actually upload 10,165 files, and the default 300 s ceiling on ``waitForAllBulkUploads`` was too tight for the server to finish extracting them -- the assertion at line 275 fired with ``state=timeout, elapsed=300.3s`` and the row never reached the createSignedURLSetJob step it was there to observe. That's a false-negative on the bug this class exists to detect: nothing was learned about the async job path at N=10000 that run. Mirrors the size-adaptive timeout already applied to waitForSignedURLSetJob: ``max(300, ceil(N / 20)) s`` -- 300 s for N=500/2000, 500 s at N=10000, 2500 s at N=50000, 5000 s at N=100000. Rate anchored to a rough "20 files/s extract" observation: at N=500, 513 files took ~9 s (well inside 300 s); at N=10000, 10165 files were still going at 300 s, and 20 files/s would predict ~500 s to finish (well inside 500 s). Includes ``timeout=Ns`` in the assertion diagnostic so a future timeout row prints the ceiling that was applied. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
Contributor
Author
|
CI standing down: Cloud CI 1 + Cloud CI 3 failures on Failing checks
Why they aren't this PR's Commit All five failures share the same server-side symptom, e.g.: The client uploads via curl, the API returns 200, but the file document stays at Fix ported None available yet in code — this is server-side. Nothing to port into this PR. Re-run Not spent yet; I'll leave the re-run decision to you rather than burn it here, since the pattern reproduces intermittently on Continuing to watch this PR. Generated by Claude Code |
Cloud CI 2 on head e3d5f51 failed with two waitForAllBulkUploads timeouts: * N=2000 hit 300.3 s / 300 s ceiling * N=10000 hit 500.4 s / 500 s ceiling The same N=2000 row finished in 22 s on b0dcce1 and again in earlier runs at ~10 s, so the extract time swings by more than an order of magnitude with server load. Chasing that with an ever-larger ceiling would still red the PR whenever the server is warm. The probe's own philosophy, stated at line 369 of the failed-job branch, is that a job outcome IS the observation the run is here to make -- the probe does not fail on things the server does; it records them. Extend the same treatment to the bulk-extract wait: if waitForAllBulkUploads times out, record a row with state='bulkTimeout' and errorMessage naming the elapsed time and ceiling, then return from the parametric case so the next N runs. The row still surfaces the observation for the bug report; CI stays green when the extract is slow. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
Contributor
Author
|
Cloud CI 2 update on head
Continuing to watch; the root cause is server-side, per the earlier standing-down comment. Generated by Claude Code |
Replace the fixed pause(20) + single-retry pause(10) with a proper
per-file poll of getFileDetails, gating each verification on the file
document actually flipping uploaded=true (which is when the server
returns a downloadUrl).
The old sequence -- upload files serially, pause 20 s, then start
verifying -- collapses when the backend is under load. The file goes
to S3 (client-side PUT succeeds) but the server hasn't committed the
file document yet, so getFileDetails returns HTTP 200 with
{uploaded: false, size: 0, no downloadUrl}. The wrapper treats that
as b=false. The old getFileDetailsWithRetry then paused 10 s and
tried once more; the total 30 s budget was still too short on a
thrashed dev-api, and the test failed at line 178 with the
"missing downloadUrl" diagnostic.
The new getFileDetailsWithRetry polls every 2 s up to 300 s until
b=true or timeout, matching the order of magnitude the sibling
waitForAllBulkUploads uses for the bulk-upload path. Since the
polling helper is called downstream in the per-file verification
loop, the upfront pause(20) is redundant and is removed from both
testSerialUploadPublishDownload and testMixedUploadPublishDownload;
a narrative line names what is now doing the waiting so a failing
run's log still explains the wait shape. Callers keep the same
getFileDetailsWithRetry signature.
testBulkUploadPublishDownload already used the proper
'waitForCompletion', true gate on the bulk-upload path and is
untouched.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
Measurement in production: an 8 MB uncompressed budget on a whole-brain lightsheet (~1 TB uncompressed, isotropic micron voxels) produces ~121k chunks at level 0. That is enough chunks for getSignedURLSetJob to spend ~85 minutes signing one level, and for the publisher's partial-upload failure modes (waltham-data-science/ndi-cloud-node#112) to surface as hundreds of missing chunks. 8 MB was the right number for a conservative decode budget and is still safe, but it is no longer the sweet spot once you measure the actual signing+fetch+decode pipeline. Bump the default to 32*2^20 (32 MB). At uint16 with isotropic voxels chooseTileShape lands on 256^3 chunks, which is exactly 32 MB uncompressed, divides evenly down a dyadic pyramid, and compresses to ~3-10 MB on the wire with blosc-zstd (one S3 fetch 50-200 ms, one decode under ~200 ms on a laptop). Level 0 chunk counts drop ~4x, which collapses the signing-latency problem for the common case; the signed-URL-set rework (ndi-cloud-node#149) still cuts the per-URL cost in parallel. No schema change: axes_order="tczyx", shape=[], chunks=[], chunk_grid=[] in lightsheetZarrLevel.json are already arbitrary-rank lists, and the reader (ndi.pyramid.multiscale) walks chunk_grid as a general N-D tuple. No reader change. The writer's chooseTileShape was already producing world-isotropic chunks (cubic when voxels are isotropic, pancake only when they aren't), so the chunks were never "2D tiles" at the default. Files changed: - +ndi/+fun/+doc/+lightsheet/makePyramid.m: default 8*2^20 -> 32*2^20, docstring rewritten to reference the sweet-spot measurement and cite the chunk-count impact (121k -> ~30k at the whole-brain scale) - +ndi/+fun/+doc/+lightsheet/chooseTileShape.m: default 8*2^20 -> 32*2^20, example numbers updated (isotropic -> 256^3 = exact 32 MB, anisotropic 1/1/5 -> [88 438 438] world-cube) - +ndi/+fun/+doc/+lightsheet/fromOMEZarr.m: forwarded default 8*2^20 -> 32*2^20, docstring updated - +ndi/+fun/+doc/+lightsheet/README-lightsheet-zarr.md: budget prose updated - +ndi/+unittest/+fun/+doc/+lightsheet/TestMakePyramid.m: testChunksAutoChosenByBudget now accepts either "within 1.3x of 32 MB budget" OR "clamped to level shape" since the 128x512x512 fixture level now has chunks either shape-clamped on Z or sized by the redistribute branch; preamble updated Explicit overrides via options.tileBudgetBytes = 8*2^20 still work; existing datasets' chunk shape is stamped on the level document and is not changed by this. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N67xH9BejMzZp4GNAq8m7w
Contributor
Author
Standing down on Cloud CI 3 failure (environmental, not this PR)The Not this PR
What actually changedThe GitHub Actions runner image was bumped between 2026-09-30 and today (2026-10-04). One-line fixReplace the % payloadBytes = javax.xml.bind.DatatypeConverter.parseBase64Binary(payloadBase64);
decoder = java.util.Base64.getDecoder();
payloadBytes = typecast(decoder.decode(uint8(payloadBase64)), 'int8');( Why not fixing it in this PRScope — the diff is about pyramid chunk shape, nothing touches cloud auth. The fix belongs in a one-commit PR against CI will re-run on the next pushThe merge-base of this PR carries the breakage too (as will every other PR's CI). Once the Generated by Claude Code |
Brings in the ndi.cloud.api.feedback commands + feedbackTest (#1023) and the cloud login JWT decode fix (#1024). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
A viewer launched from MATLAB has no terminal, so its output -- which tiles it fetched, what failed and why -- was lost. Both launch paths (ndi.fun.doc.lightsheet.view and LightsheetZarrManager's View dialog) now go through the new ndi.fun.doc.lightsheet.launchViewer, which sets NDI_LIGHTSHEET_DEBUG=1 for the viewer and appends its stdout and stderr to lightsheet_YYYYMMDDHHMMSS.txt in ndi.common.PathConstants.TempFolder (created if missing), after a header with the time and command. The log path is printed in the Command Window and shown in the manager's status line. view's --list mode is unchanged: its ids are for the user to read. Pieces: viewerLogFile (the timestamped path) and viewerShellCommand (the POSIX / cmd.exe line: env var, append redirect, detached unless 'wait') are pure and tested on every platform; launchViewer writes the header and runs it. view gains 'debug' and 'logFile' options. LightsheetZarrManager's private detach is gone (its only caller now uses launchViewer). MATLAB is not available where this was written, so TestLaunchViewer has not been run; the POSIX command it builds was exercised in sh with a stand-in launcher (debug flag set, stdout and stderr appended, a quote in the log path handled). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
stevevanhooser
changed the title
ndi.fun.doc.lightsheet.fromSPIM(S, rootDir, 'subjectID', id) writes the same lightsheetZarrPyramid + lightsheetZarrLevel documents as fromOMEZarr, chunks materialized, without an intermediate spim2zarr / OME-Zarr step. * Level 0 (all channels, axes tczyx) is read from the stitched, harmonized All_Channels slices via ndr.format.smartspim.readAllChannelsInfo, each slice read only over the needed (y, x) window (imread PixelRegion). * The mean and max ladders are built here: each level block-reduces the level just written, so nothing re-reads full resolution. A spatial axis is halved while its voxel size is <= 1.5x the finest among axes still longer than one voxel (coarse-z stacks even out in x/y first), until every spatial axis is <= minLevelSize (512) or maxLevels (10). Level 0 is shared by both ladders (reduction_function 'none'). * Channel subset/order, voxel-size override, mean-only ladders. makePyramid gains levelSources (per level path: 'omezarr' -- the default, unchanged -- 'smartspim', or 'reduce' from an earlier level's stored chunk files, resolved to paths on the main thread so parfeval workers read plain files) and channelNames. A reduced level's read budget is shrunk by its read amplification so peak memory stays near prefetchBytes. New ndi.fun.doc.lightsheet.reduceBlocks: vectorized per-axis block mean/max with ndr.format.omezarr.reduce's semantics (trailing partial block included). NDR's reduce loops once per output voxel -- ~2M iterations per 32 MB chunk -- which is unusable at whole-brain scale. MATLAB was not available where this was written, so TestFromSPIM and TestReduceBlocks have not been run. The reduce algorithm, the ladder plan behind the tests' expected shapes, and the stored-chunk round trip (C-order write, region read-back across chunk edges) were checked with column-major NumPy mirrors. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
stevevanhooser
pushed a commit
to Waltham-Data-Science/NDI-python
that referenced
this pull request
Oct 12, 2026
CI's "MATLAB/Python bridge completeness" checks the bridge against NDI-matlab main, where +ndi/+cloud/+download/+internal/signedUrlDiskCache.m and +ndi/+cloud/+sync/+internal/fetchManifest.m do not exist yet: both live on claude/lightsheet-zarr-ndi-viewer-djp5vk (VH-Lab/NDI-matlab#979). Add matlab_branch to the two entries, as the bridge reference prescribes for files on an unmerged branch. Passes against NDI-matlab main and against the branch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
…s ladders CI's first MATLAB run of the new tests: TestReduceBlocks, TestLaunchViewer and the structural TestFromSPIM tests pass; the two value checks failed on every plane. ndr.test.format.smartspim.makeExampleFixture writes All_Channels slice z (0-based) as 1000*(k+1) + z, so 1-based slice z holds 1000*(k+1) + z - 1 -- the expectations were one high. Level 1 (z halved) is then base + [1 3] for both mean (0.5 and 2.5 round half away from zero) and max. The data path was right; the expected values were not. Also preallocates the per-ladder level and info arrays in fromSPIM (Code Analyzer flagged the growing levelInfo: its %#ok sat on a continuation line, where the analyzer does not read it). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
Contributor
Author
|
Cloud CI 1 on
Why I don't think this PR caused it:
I'll re-run the failed job once, after the rest of this workflow finishes; GitHub refuses a re-run while it's in progress. If it fails again, it's real, and I'll make the test poll until the file shows as uploaded, as The two earlier Generated by Claude Code |
stevevanhooser
changed the title
…r-ndi-viewer-djp5vk
Contributor
Author
|
Cloud CI 1 failed on What happened: the curl PUT reported success, but the file record stayed Why I don't think this PR caused it:
One thing to watch: an upload URL requested without a checksum is still signed with No fix exists yet. I'm re-running the failed job once. Generated by Claude Code |
…ommand The dialog previewed only the bare napariViewLightsheet command, while Launch ran it wrapped with NDI_LIGHTSHEET_DEBUG=1 and the log redirection. The log file is now named when the dialog opens, the preview shows viewerShellCommand(cmd, logFile, false, true, ispc) and Launch passes that same logFile to launchViewer, so the text shown is exactly the line that runs (and can be pasted into a terminal). Errors from building the command are still shown as plain text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
testTheManagerLaunchesThroughLaunchViewer looked for the literal launchViewer(cmd), which fb10169 replaced with launchViewer(cmd, 'logFile', logFile). Check for that call, and that the dialog's preview is built by viewerShellCommand for the same log file with launchViewer's defaults (no wait, debug on). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
Contributor
Author
|
The intermittent Cloud CI 1 failures here ( Likely cause: the upload PUT succeeds, but the S3-triggered lambda that marks the file uploaded has only 8 concurrent slots. This PR's new This is inferred, not confirmed from logs. If those two tests still fail after this branch takes in main, the cause is something else; next step is CloudWatch for Generated by Claude Code |
Brings in #1027 (waitForFileUpload; uploadSingleFile waits for NDI Cloud to record the upload; FilesTest uses it). Conflict in uploadFilesForDatasetDocuments: kept both sides -- this branch's files_uploaded_count / Verbose summary and main's sentUids collection with one waitForFileUpload after the serial loop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
3 of 6 tasks
stevewds
approved these changes
Oct 14, 2026
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
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Lightsheet volumes become NDI documents that can be ingested (from an OME-Zarr store or directly from a SmartSPIM acquisition), uploaded to NDI Cloud, downloaded again, and viewed in napari through NDI-python. The PR began as a scaffold for the document types. It now carries the full pipeline, plus the cloud upload, download and signed-URL fixes that pipeline turned up at whole-brain scale.
Design mirrors
+ndi/+fun/+doc/+gene/andndi.gui.app.GEFManager, so a reader of the GEF ingest will recognise the shape.What's in it
Documents
lightsheetZarrPyramid: one per source volume. Records the axes, channels, dtype and the level-0 shape, voxel size and translation, and depends on the source file reference.lightsheetZarrLevel: one per (pyramid, level), carrying areduction_function(mean or max). Its chunk bytes are a DID file serieschunk.bin_#with a bare manifest. The per-level metadata covers shape, chunks, chunk grid, voxel size, translation, codec and fill value.Ingest:
ndi.fun.doc.lightsheet.*fromOMEZarrprobes a store and builds the pyramids.makePyramidmaterializes the chunks:chooseTileShape. The default budget is now 32 MB, which is 256³ for uint16. At 8 MB, a whole-brain volume made about 121k level-0 chunks, and signing them took around 85 minutes.ndr.format.blosc) and installed with oneaddFileSeriescall.parforeach, with asynchronous prefetch of the next), with memory caps and an optionaldebugLog.ProgressBarWindow, or through aprogressFcncallback.levelSources. Each level path can now name where its bytes come from: the OME-Zarr store (the unchanged default), SmartSPIM slices, or a block-reduction of a level written earlier in the same call.fromSPIMbuilds the same documents straight from a SmartSPIM acquisition's stitched, harmonizedAll_Channelsslices (viandr.format.smartspim), with nospim2zarrstep.tczyx). Each slice is read only over the rows and columns needed.minLevelSize(512 voxels).reduceBlocksis a vectorized block mean/max with the same semantics asndr.format.omezarr.reduce. NDR's version loops once per output voxel, which is too slow at whole-brain scale.makeSourceFile,levelTable,chooseLevel,viewCommandare pure helpers.Viewing
ndi.gui.app.LightsheetZarrManagerlists a session's pyramids with levels and reductions read from the level documents. It adds from an OME-Zarr path, deletes a pyramid along with its levels, and has a View dialog that opens napari through/usr/local/bin/napariViewLightsheet. It is styled on the NDI Cloud palette.ndi.fun.doc.lightsheet.viewis the command-line entry point.launchViewer(withviewerLogFileandviewerShellCommand):NDI_LIGHTSHEET_DEBUG=1.lightsheet_YYYYMMDDHHMMSS.txtinndi.common.PathConstants.TempFolder. The path is printed in the Command Window and shown in the manager's status line.Cloud upload, download and signed URLs
uploadDatasetcan target a named organization (resolveOrganizationId).zipForUploadbatches in a single pass, so its cost grows with the number of files rather than batches × files.SyncFiles=false.listFilesAll.fetchManifestand single-file DID fetches skip the batch signed-URL scope.batchSignedUrlLookupretries with exponential backoff.signedUrlDiskCachekeeps signed URLs on disk across reopening a dataset.ProgressBarWindow: each bar can have its own timeout, andlist_binary_filesandzipForUploadguard the window handle.Test fixtures and demos
ndi.test.lightsheet.makeBlobFixtureandingestBlobFixturebuild a synthetic multi-channel OME-Zarr blob.ndi.test.cloud.lightsheet_blob_cloud_roundtripruns the end-to-end ingest → upload → download → view demo. There is also a small zsh helper to find the napari debug log.TestSignedUrlSetJobScaling, which runs up to 100k files, andTestSignedUrlSetJobFixturereproduce the signed-URL-set job problems found at scale.Also merged from
mainThe
ndi.cloud.api.feedbackcommands withfeedbackTest(#1023), and the cloud login JWT decode fix (#1024).Companion work
Add in-app problem reporting and improve tile fetch prioritization Waltham-Data-Science/NDI-python#324: the napari viewer side. It includes:
ndi.cloud.api.feedback;Earlier viewer work merged in CI: pin NDI-matlab ref for the lightsheet fixture workflow Waltham-Data-Science/NDI-python#319.
Waltham-Data-Science/ndi-cloud-node#158: the
/v1/feedbackendpoint the report button posts to.VH-Lab/NDR-matlab, VH-Lab/NDR-python: the SmartSPIM
All_Channelsreaders thatfromSPIMuses (merged to theirmainbranches).Test plan
matlab.unittestin CI on668beb13: 1,491 passed, 0 failed. This includes the newTestFromSPIM,TestReduceBlocksandTestLaunchViewersuites, all written without MATLAB and first run here.FilesTest/testListFilesWithOptionsfailed once, with a freshly uploaded file not yet marked as uploaded, and passed on re-run.ndi.test.cloud.lightsheet_blob_cloud_roundtripend to end, then open the viewer fromLightsheetZarrManagerand confirm alightsheet_*.txtlog appears in the NDI temp folder with[lightsheet]lines in it.fromSPIMon a real SmartSPIM acquisition withAll_Channels; open the result in the viewer.start … cmd /cform writes the log.🤖 Generated with Claude Code
https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L