Skip to content

ink inference: optional vc_reader backend for full-3D patch reads - #2226

Closed
phippsrtyler wants to merge 3 commits into
ScrollPrize:mainfrom
phippsrtyler:ink-vc-reader
Closed

phippsrtyler wants to merge 3 commits into
ScrollPrize:mainfrom
phippsrtyler:ink-vc-reader

Conversation

@phippsrtyler

@phippsrtyler phippsrtyler commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

In one sentence: infer_full3d_tifxyz --reader vc_reader loaded remote full-3D patches about 2x faster in my runs (36.6 → 18.1 s for 45 patches of PHerc Paris 4) by reading each DataLoader batch with one vc_reader read_regions call. The patches are bit-identical, and the default zarr reader is unchanged.

One real example: 45 planned 128³ patches (overlap 0.5) on the public PHerc Paris 4 7.91 µm volume over HTTPS. The inference DataLoader used batch 8, 4 spawn workers and prefetch factor 2. A pass took 36.6 s with the default zarr reader and 18.1 s with --reader vc_reader (median of 3 interleaved rounds).

Before:

  • NativePatchDataset read every patch with its own read_bbox_with_padding zarr slice.
  • With overlap 0.5, a remote chunk was fetched again for every patch that touched it, one patch at a time.

After this PR:

  • volume_io.open_vc_reader opens one level with villa's vc_reader.
    • It sets a memory cache and an HTTP connection limit.
    • It imports lazily and raises a clear error if the package isn't installed.
  • volume_io.read_bboxes_with_padding reads several bboxes with the same padding contract as read_bbox_with_padding.
    • On a vc_reader volume it makes one read_regions call, so shared chunks are read once and in parallel.
    • Otherwise it loops over read_bbox_with_padding.
  • NativePatchDataset changes:
    • It gains __getitems__, which DataLoader calls once per batch.
    • It gains reader, reader_connections and reader_memory_cache_mb (defaults zarr, 32, 512).
    • __getitem__ is now the one-item case of __getitems__.
  • New CLI flags: --reader {zarr,vc_reader}, --reader-connections, --reader-memory-cache-mb.
    • The CLI fails at startup if --reader vc_reader is given without the package.
  • vc_reader stays optional. It is not added to dependencies.

Proof:

Machine, volume Patches --reader zarr --reader vc_reader speedup
PC CPU (i9-13900K, WSL, home internet), PHerc Paris 4 7.91 µm, HTTPS 45 × 128³, batch 8, 4 workers 18.6 s / 16.9 s (two runs) 10.1 s / 10.0 s 1.8x / 1.7x
Server CPU (Threadripper PRO 3945WX, shared, load average 10.5–17), same volume 45 × 128³, batch 8, 4 workers 36.6 s (25.82, 37.98, 36.63) 18.1 s (13.03, 18.13, 21.22) 2.0x
  • Parity:
    • Every patch from both readers equalled a stock crop saved from main: 45/45 for zarr and 45/45 for vc_reader.
    • DataLoader batch sums matched across all 6 timed passes.
  • Tests:
    • test_read_bboxes_with_padding_matches_single_reads uses a stub read_regions volume that fills with 7 and returns views. It checks padding, valid slices and a single call against read_bbox_with_padding.
    • test_vc_reader_matches_zarr_reads reads a local zarr through real vc_reader, including boxes partly outside it. It is skipped when vc_reader is absent.
    • test_open_vc_reader_without_package_says_how_to_fix checks the error when the package is missing.
    • test_native_dataset_vc_reader_batches_match_zarr_reads checks that DataLoader batches match zarr reads, with one read per batch.
    • test_native_full3d_inference + test_volume_io: 30 passed with the latest commits.

Why / where this is useful:

Training and running 3D ink models straight from raw, unflattened CT means streaming many overlapping 128³ patches from a remote scroll zarr, and the loader is the bottleneck. On my PC, the 45-patch inference pass went from about 18 s to 10 s with --reader vc_reader (1.7–1.8x, identical patches, two quiet repeats). This PR lets the full-3D inference loader use villa's fast reader with one flag, and the same helper is ready for training datasets.

  • I personally verified that the example and proof above were produced by this PR on the stated data.

Details

  • Rounds and load:
    • Rounds alternated reader order: zarr first, then vc_reader first, then zarr first.
    • Runs were CPU only on a shared server, with load average 10.5–17 during the timed rounds.
    • vc_reader won every round, by 1.7–2.1x.
  • Earlier measurements: On the same 45 patches without a DataLoader, a single process with 512 MB cache and 32 connections took 8.2–8.5 s against 49.4 s per-patch on main. connections=32 and the 512 MB memory cache are the measured best settings.
  • Workers: Each DataLoader worker opens its own vc_reader. Memory cache and connections are per worker.
  • Flaky HTTP:
    • vc_reader retries each remote GET (5 retries).
    • In the same A/B run, the default zarr reader failed once with an aiohttp TimeoutError, and an earlier sweep failed on a DNS error. vc_reader did not fail in these runs.
  • Training, not in this PR:
    • The random-patch CoordPatchDataset is built inside a ConcatDataset, which does not forward __getitems__. Per-batch reads only gained 1.18x there (67.0 → 57.0 s, 64 random 128³ patches, noisy).
    • For training, vc_reader's own prefetch_window is the better tool. In a standalone loop it gave 1.5–1.8x over plain read_regions.
  • Not covered:
    • A volume path that is itself the level array (vc_reader appends the level).
    • open_volume's auth JSON and zarr2 fallback are not applied on the vc_reader path.

@vercel

vercel Bot commented Oct 22, 2026

Copy link
Copy Markdown

@phippsrtyler is attempting to deploy a commit to the scroll Team on Vercel.

A member of the Team first needs to authorize it.

--reader vc_reader reads each DataLoader batch of planned patches with one
vc_reader read_regions call, so chunks shared by overlapping patches are
fetched once and in parallel. The default zarr reader is unchanged.
@phippsrtyler

phippsrtyler commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor Author

This one's done on my end, with proof rows on 3 cards. I'm at the 3-open-PR limit, so I can't mark it ready. Happy for it to go into review whenever a slot opens, or I can close one of my others if you'd rather review this first.

@bruniss

bruniss commented Nov 4, 2026

Copy link
Copy Markdown
Member

handled in #2440

@bruniss bruniss closed this Nov 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants