Skip to content

fix(render): OME-Zarr scale metadata must describe the render level, not level 0 - #1229

Open
Bullo27 wants to merge 1 commit into
ScrollPrize:mainfrom
Bullo27:fix/zarr-scale-render-level
Open

fix(render): OME-Zarr scale metadata must describe the render level, not level 0#1229
Bullo27 wants to merge 1 commit into
ScrollPrize:mainfrom
Bullo27:fix/zarr-scale-render-level

Conversation

@Bullo27

@Bullo27 Bullo27 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

vc_render_tifxyz renders from pyramid level --group-idx, so one output voxel spans 2^g native voxels. The TIFF path accounts for that; the OME-Zarr path does not, and the two outputs of a single run contradict each other.

The contradiction

Same segment, same volume, same invocation, --group-idx 2, output 470×470:

output declared scale
--tif-output XResolution 139.46 DPI → 182.128 µm/px
--zarr-output .zattrs scale → 45.532 µm/px

Every --group-idx reported 45.532 regardless of render size (1880² / 940² / 470²), so anything reading the multiscale metadata — napari, neuroglancer, any downstream measurement — gets physical dimensions wrong by 2^g.

This is not specific to remote volumes. A local volume rendered at --group-idx > 0 is affected identically; it's purely about the pyramid level.

Why

The TIFF branch computes the rendered voxel size:

const double voxelSizeAtRenderLevelUm = ds_scale > 0
    ? voxelSizeUm / double(ds_scale)
    : voxelSizeUm;
tifDpi = voxelSizeToDpi(voxelSizeAtRenderLevelUm);

but both writeZarrAttrs(...) call sites pass the raw native base_voxel_size. writeZarrAttrs does receive groupIdx, and uses it only to record attrs["source_group"] — never for the scale, which is computed as sYX = baseVoxelSize * 2^l, sZ = baseVoxelSize.

The intent is already documented in buildOffsetList:

Offsets are distances along the unit surface normal, in level-g voxels … Do NOT scale by ds_scale here — that would make the through-normal spacing the native/level-0 resolution while the in-plane spacing is level-g, yielding an anisotropic stack that mismatches the isotropic .zattrs scale.

So the stack is deliberately isotropic in level-g voxels, and .zattrs has to describe level-g spacing. It was describing level-0.

The change

Compute the rendered voxel size once, next to the TIFF calculation, and pass it to both call sites.

Verification

Against the live open-data bucket (PHercParis4, 45.532 µm native), rendering the same segment at each level and reading back both outputs:

--group-idx render px .zattrs µm TIFF µm/px expected
0 1880 45.532 45.532 45.532
1 940 91.064 91.064 91.064
2 470 182.128 182.128 182.128
3 235 364.256 364.256 364.256

Before the change the .zattrs column read 45.532 at every level. Built on Ubuntu 24.04 (gcc, Ninja/Release); full suite passing.

One related thing I left alone

sZ still ignores slice_step. With the default (1.0) that's correct, but --slice-step 2 puts layers two level-g voxels apart while .zattrs still claims one. writeZarrAttrs already receives sliceStep and records it in attrs["slice_step"], so the information is there. I kept it out of this diff to keep the change focused — happy to fold it in if you'd like it fixed here.


Found by differential-testing the renderer against the raw zarr (which, separately, came out exact — corr 0.9999 on pixel values at levels 0 and 1). Investigation and patch with Claude Code (Opus 5); every number above is from an actual run.

vc_render_tifxyz renders from pyramid level --group-idx, so one output voxel
spans 2^g native voxels. The TIFF path accounts for this
(voxelSizeAtRenderLevelUm = voxelSizeUm / ds_scale) but both writeZarrAttrs
call sites passed the raw native base_voxel_size, and writeZarrAttrs receives
groupIdx only to record it as "source_group" -- never for the scale.

The two outputs of a single run therefore contradicted each other. Same
segment, same volume, --group-idx 2, output 470x470:

  --tif-output    XResolution 139.46 DPI  -> 182.128 um/px   (correct)
  --zarr-output   .zattrs scale           ->  45.532 um/px   (4x off)

Every group-idx reported 45.532 regardless of render size, so anything reading
the multiscale metadata -- napari, neuroglancer, downstream measurement --
got physical dimensions wrong by 2^g. This is independent of remote volumes;
a local volume at --group-idx > 0 is affected identically.

Compute the rendered voxel size once and pass it to both call sites. The
buildOffsetList comment already states the intent: the stack is isotropic in
level-g voxels, so .zattrs has to describe level-g spacing.

Verified against the live open-data bucket (PHercParis4, 45.532 um native),
zarr and TIFF now agreeing at every level:

  g0  1880px   45.532 um   g1  940px   91.064 um
  g2   470px  182.128 um   g3  235px  364.256 um

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WKT3CfY4msFhzjd1cXzbrp
@vercel

vercel Bot commented Jul 25, 2026

Copy link
Copy Markdown

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

A member of the Team first needs to authorize it.

@Bullo27

Bullo27 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

One implication I should state explicitly, since it isn't only forward-looking: any OME-Zarr already rendered at --group-idx > 0 carries a scale that is wrong by 2^g. The pixel data is fine — this is purely the .zattrs multiscale metadata — but anything that derived a physical measurement from it is off by that factor.

Scoping it as far as I can from outside:

  • --tif-output was always correct, so only --zarr-output runs are affected.
  • --group-idx 0 was always correct, so only downsampled renders are affected.
  • --group-idx is a required argument, so nobody lands on a non-zero level accidentally.
  • The tutorial in docs/07_tutorial5.md uses --group-idx 0 --tif-output, so the documented path is clean.

What makes this worth flagging rather than just fixing is the size of the error. A factor of 2 or 4 is plausible enough to pass a sanity check — unlike, say, the 45,532× discrepancy in #1228, which would be obvious the moment anyone looked at it. A 2× error in a sheet measurement or an ink-density-per-cm² number would very likely go unnoticed.

I have no way to tell from outside how many people render downsampled into zarr, so I'm not in a position to judge whether that warrants any wider notice — you'd know. If it does, the affected outputs are identifiable without re-rendering: .zattrs records source_group, so any output where source_group > 0 has a scale that needs multiplying by 2^source_group. The pixels don't need to be regenerated, just the metadata corrected.

Claude Code (Opus 5).

@Bullo27

Bullo27 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

Concrete numbers for the "full suite passing" line above, since CI doesn't run on fork PRs here:

100% tests passed, 0 tests failed out of 109
Total Test time (real) =  31.96 sec

Ubuntu 24.04, gcc, Ninja/Release, -DVC_TESTING=ON, on this branch.

Claude Code (Opus 5).

@bruniss

bruniss commented Jul 25, 2026

Copy link
Copy Markdown
Member

This appears to be a metadata only issue, but worth fixing

@Bullo27

Bullo27 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

Agreed — metadata only in the strict sense that matters most: no pixel is affected, so nothing already rendered needs re-rendering. And since .zattrs records source_group, an existing output can be corrected in place — the error is a uniform factor of 2^source_group across every scale entry, so it's a few lines over the .zattrs rather than a re-run.

The one thing I'd still like a call on is the slice_step case at the end of the description: sZ ignores it, so --slice-step 2 declares one-voxel layer spacing when the layers are actually two apart. Same class of bug and a one-line fix, but it touches a second flag's behaviour, so I kept it out to leave this diff single-purpose. Happy to fold it in here or file it separately — whichever you'd rather review.

Claude Code (Opus 5).

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