Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong chunk loaded at low-resolutions #60

Open
joshmoore opened this issue Dec 3, 2020 · 13 comments
Open

Wrong chunk loaded at low-resolutions #60

joshmoore opened this issue Dec 3, 2020 · 13 comments

Comments

@joshmoore
Copy link

For http://hms-dbmi.github.io/vizarr?source=https://s3.embl.de/i2k-2020/em-raw.ome.zarr @tischi reported that the browser hangs at this stage:

Screen Shot 2020-12-03 at 10 05 01

@will-moore pointed out that "it seems even at very low resolution, (where you only have 1 tile) we are getting 404 https://s3.embl.de/i2k-2020/em-raw.ome.zarr/s6/0.0.1351.0.0"

even though:

(z) ~ $ome_zarr info https://s3.embl.de/i2k-2020/em-raw.ome.zarr/
https://s3.embl.de/i2k-2020/em-raw.ome.zarr/ [zgroup]
 - metadata
   - Multiscales
 - data
   - (1, 1, 2854, 3240, 3438)
   - (1, 1, 1427, 1620, 1719)
   - (1, 1, 714, 810, 860)
   - (1, 1, 357, 405, 430)
   - (1, 1, 179, 203, 215)
   - (1, 1, 90, 102, 108)
   - (1, 1, 45, 51, 54)
https://s3.embl.de/i2k-2020/em-raw.ome.zarr/labels/ [zgroup] (hidden)
 - metadata
   - Labels
 - data
https://s3.embl.de/i2k-2020/em-raw.ome.zarr/labels/cells/ [zgroup] (hidden)
 - metadata
   - Multiscales
 - data
   - (1, 1, 2854, 3240, 3438)
   - (1, 1, 1427, 1620, 1719)
   - (1, 1, 714, 810, 860)
   - (1, 1, 357, 405, 430)
   - (1, 1, 179, 203, 215)
   - (1, 1, 90, 102, 108)
   - (1, 1, 45, 51, 54)
   - (1, 1, 23, 26, 27)
@manzt
Copy link
Member

manzt commented Dec 3, 2020

Hmmm let me have a look. First thing that stands out to me is that the axis slider is labeled with a 2 and not Z, which likely means vizarr doesn't recognize the multiscale image as OME-Zarr.

@joshmoore
Copy link
Author

Related?

Unhandled Promise Rejection: TypeError: undefined is not an object (evaluating 'source.keyPrefix')
https://s3.embl.de/i2k-2020/em-raw.ome.zarr/s2/0.0.0.0.0

...

luma.gl: Found instanced attributes on non-instanced model – "XR-Static-Layer-0-64-64-0-6-Background-Image-zciorf4e6yg"
https://s3.embl.de/i2k-2020/em-raw.ome.zarr/s3/0.0.0.1.0

@manzt
Copy link
Member

manzt commented Dec 3, 2020

Thanks. I think the first is related. I need to inspect the metadata for the arrays, but my guess is that this is related to the chunk_shape.

We currently rely on all non-xy dims having a chunk_shape of 1. My best guess is that these data are chunk-optimized for 3D (e.g. 64x64x64).

Update -- this is the issue. We should catch this earlier in vizarr and provide a more helpful error.

$ curl -L https://s3.embl.de/i2k-2020/em-raw.ome.zarr/s2/.zarray
{
  "chunks": [
    1,
    1,
    96,
    96,
    96
  ],
  "compressor": {
    "blocksize": 0,
    "clevel": 5,
    "cname": "lz4",
    "id": "blosc",
    "shuffle": 1
  },
  "dtype": "|u1",
  "fill_value": 0,
  "filters": null,
  "order": "C",
  "shape": [
    1,
    1,
    714,
    810,
    860
  ],
  "zarr_format": 2
}

@tischi
Copy link

tischi commented Dec 3, 2020

We currently rely on all non-xy dims having a chunk_shape of 1. My best guess is that these data are chunk-optimized for 3D (e.g. 64x64x64).

Yes, they are chunked in 3D. That explains why vizarr was trying to load too large chunk indices in the z-dimension for the lower resolutions.

Is that something that's easy for you to fix?

@manzt
Copy link
Member

manzt commented Dec 3, 2020

Is that something that's easy for you to fix?

Good question. This is a limitation of the chunk loaders we've written for Viv, the library that powers vizarr, which we've designed to support 2D downsampling (e.g. current output of bioformats2raw). Short answer is that support would likely require a substantial change to Viv and is not a super easy fix. (We have two different loaders in Viv, one for OME-TIFF and Zarr, but both are based on the old OME-TIFF style of 2D image stacks, which make them interchangeable to our rendering utilities.)

With that said, the types of images we support is really a function of what are available pubically via HTTP. We've been able to iterate rapidly on OME-Zarr mostly because @joshmoore and co have done such a wonderful job putting up examples on the web. Will this image remain available? I know for a fact @ilan-gold is eager to work with more 3D data.

For example, I was able to prototype a rough idea in a few minutes this morning. We just don't have many (any!) 3D Zarr examples to play with.

Screen Recording 2020-12-03 at 9 37 27 AM

@manzt
Copy link
Member

manzt commented Dec 3, 2020

From discussion on slack:

@tischi : Maybe Trevor Manz’s code is not yet dealing with missing chunks? Just a guess, because we had similar issues initially with other code.

This is a great guess, and I think exactly what is going on. I just realized a recent PR in Viv removed this behavior (hms-dbmi/viv#280) by accident. I'll open an issue.

@will-moore Seems even at very low resolution, (where you only have 1 tile) we are getting 404 https://s3.embl.de/i2k-2020/em-raw.ome.zarr/s6/0.0.1351.0.0

This is related to the above, and unfortunately there is no way (that I know) to remove those 404s in the console. The HTTPStore implementation makes a request for a chunk and will throw a KeyError if res.status === 404.

The ZarrArray implementation tries to eagerly request a chunk from a store, and if a KeyError is raised, will return a zero-filled array: https://github.com/zarr-developers/zarr-python/blob/a5dfc3b42374619e1d941d0d4991f95b8c01ddcf/zarr/core.py#L707-L716

For performance reasons (and to avoid copying data in JS), the recent PR in Viv makes a direct call to the store to decode a chunk but I forgot to check if a KeyError is thrown.

@tischi
Copy link

tischi commented Dec 3, 2020

Will this image remain available? I know for a fact @ilan-gold is eager to work with more 3D data.

I can leave this image there for you to develop and we (at EMBL) will have a lot of 3D data use cases.
It is essential for us that arbitrary chunking in 3D is supported and it would be amazing if vizarr would do it!
So, from EMBL's side super much appreciated and probably many people very happy if you work on this!

@will-moore
Copy link
Collaborator

Just looking at this again, I'm seeing a 503 Error from https://s3.embl.de/i2k-2020/em-raw.ome.zarr/.zgroup
@tischi - Is this image still available? Thanks.

@tischi
Copy link

tischi commented Jan 8, 2021

@will-moore It should be still there but I think they have a general problem with the S3 storage at EMBL right now.
Please keep trying, maybe again on Monday.

@tischi
Copy link

tischi commented Jan 8, 2021

@will-moore Could you try again now?

@will-moore
Copy link
Collaborator

Thanks - the top level .zgroup is working so I can view something in vizarr. Some chunks are quite slow to load (30 - 60 secs) but that might be my network, and there are some 404s (e.g. https://s3.embl.de/i2k-2020/em-raw.ome.zarr/s2/0.0.0.0.0) but I haven't looked enough yet to know whether they should actually exist or if that's just vizarr needs fixing?

Also just wondering what the UI behaviour should be when you zoom in/out? E.g. the lowest resolution has 2853 Z-sections, so you'd expect the Z-slider to have 2853 steps at lowest resolution. But when you zoom out and have fewer Z-sections, do we expect the Z-slider to change? That would seem like a fair bit more work.
Or do we map e.g. Z-slider position of 2853 -> Z-index of 2853 at resolution 0 -> Z-index of 1427 at resolution 3 -> Z-index of 1 at resolution 6 etc? If this happens in viv then maybe vizarr wouldn't even know what the size-Z is at the current zoom level?

Any thoughts @manzt? Is that the approach you were prototyping above?
I'd be interested in trying to get this working if you have time to give me pointers (maybe your prototype code would be a good start)??

Many thanks!

@tischi
Copy link

tischi commented Jan 8, 2021

and there are some 404s

The specification of n5 allows for missing chunks. I think ome.zarr as well?!

@manzt
Copy link
Member

manzt commented Jan 8, 2021

Any thoughts @manzt? Is that the approach you were prototyping above? I'd be interested in trying to get this working if you have time to give me pointers (maybe your prototype code would be a good start)??

I'll try to look for the branch where I hacked around on this – it was just a quick hack to figure out what was going on. If I remember correctly, there were a couple of issues:

import { slice } from 'zarr';
import { ZarrLoader as _ZarrLoader } from '@hms-dbmi/viv';

class ZarrLoader extends _ZarrLoader {

  async getTile({ x, y, z, loaderSelection }) {
    const source = this._getSource(z); // returns ZarrArray (z is pyramidal level)
    const [xIndex, yIndex] = ['x', 'y'].map(k => this._dimIndices.get(k)); // returns axis index for x and y

    const dataRequests = loaderSelection.map(async sel => {
      const selection = this._serializeSelection(sel); // ensures number[]
      selection[yIndex] = slice(y * this.tileSize, (y + 1) * this.tileSize); // contiguous y slice
      selection[xIndex] = slice(x * this.tileSize, (x + 1) * this.tileSize); // contiguous x slice 
      // NOTE: Zarr will throw if you index larger than shape. We used chunks before 
      // because they were consistent in size (even edges). You'll need to adjust the 
      // size of slices above if they are edge tiles.
      
      // selection is now ~ [number, number, number, slice(), slice()];
      return source.getRaw(selection);
    });
    const data = await Promise.all(dataRequests);
    const { shape: [height, width] } = data[0]; // get dims from first image
    return { 
      data: data.map(d => d.data), // extract TypedArray data
      width, 
      height 
    };
}
  • 2.) For the Z-slider, I think I downsampled what ever the UI showed for loader loaderSelection depending on the zoom level. Don't think it's the right approach.

@will-moore, hopefully you can get started by protoyping this all in vizarr. It could be that vizarr requires a more sophisticated Loader than most viv applications, but we could also think about moving this code to Viv so it is reusable (if we come up witha good solution!).

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

No branches or pull requests

4 participants