-
Notifications
You must be signed in to change notification settings - Fork 10
ENH: for multiscale, use dask and auto-rechunk #45
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
base: main
Are you sure you want to change the base?
Conversation
|
Should the plugin just reduce the dask setting? and let it auto choose then? This still isn't great, because it keeps superwide shape, much wider than any display e.g. This is better than using native chunks, but still not great. |
|
After some discussion on zulip: https://napari.zulipchat.com/#narrow/channel/212875-general/topic/lazy.20reading.20tiff-based.20WSI/near/518694288 |
|
For CMU-1 With |
|
One issue is that dask array performance to get a single chunk out is quite bad when the overall task graph has many (say millions) of chunks. afaik that issue remains unfixed. So I would probably err on the side of slightly bigger chunks, say 4MiB. How does that work for you performance-wise @psobolewskiPhD? Otherwise I think it's a good idea to merge and release this quickly. |
|
I'll try to test ASAP, but I don't have a good idea for a real test other than eyeballing. |
|
I like your eyeball tests! You have a good eye! 😊 |
|
(at any rate, I'm happy for you to self-merge as-is to get you unblocked. We can always iterate!) |
|
(and you could turn my comment into an issue. 😊) |
|
I only have the smaller sample svs and ndpi. 4 MiB The question is how likely is a 3D multiscale? because my understanding is for 3D you want smaller chunks. The full size ones are ~60 GiB uncompressed? so then at 1 MiB it's 60K chunks, at 3 MiB it would be 20K chunks... |
Anything above 200ish GB I start to consider 'bigger than we can expect to work smoothly out of the box', so even with any of the chunk sizes mentioned, it would still be at most 200k chunks (with 1MiB), so I think whatever y'all deem reasonable would find me content.
I think think this is the direction light sheet is going -- but I have very little experience. |
I would assume light sheets are not using TIFF variants, but something like zarr or n5? I'm tempted to leave this at 1 MiB for now, which was recommended by multiple people much smarter than me. And we can always adjust it upwards if the need arises. This might all become moot if we want to move to a zarr v3 world. |
|
Agreed! Thanks for all your work on this @psobolewskiPhD ! I'm on phone but I'll try to remember to merge and release in the morning. |
|
Making draft until after #48 is merged. |
|
Ok, we're going to need this.
It's probably at least partially related to: :freeze: With wrapping the arrays in a And now it's pretty alright with zarr3. Still worse than the equivalent with zarr2, but the user experience is passable for sure. |
|
@psobolewskiPhD I'm trying to wrok on these zarr 3 performance issues. Just so i understand the simplest reproducer here is to download a large tiff from openslide and open it up with napari-tiff main branch prior to this PR? |
|
Hi @ianhi thanks for reaching out! You may also want to look at the reproducers from cgohlke here: |
|
Implementing this is a workaround for:
Of course, merging back doesn't work: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #45 +/- ##
==========================================
+ Coverage 79.95% 80.00% +0.04%
==========================================
Files 8 8
Lines 459 460 +1
==========================================
+ Hits 367 368 +1
Misses 92 92 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Bumping this. I did some tests with the latest napari, tifffile, zarr, dask and still get the same behaviors, where using dask with 1MiB chunks is massively more performant for the very large NDPI i have access to with the "pathological" native chunks. Oddly, just doing some |
Currently the plugin uses the zarr tifffile backend to read multiscale tiff-variants, which includes ndpi, svs, and other whole-slide image formats. Note: this is only supported with zarr<3)
In this PR, I wrap the arrays with dask, which has two primary advantages:
(chunks='auto')chunks='1 MiB'with a 1 MiB chunk mass, which is a major performance boost for ndpi images from Hamamatsu slide scanners which have native chunks of (8, 3840, 3). You can download some samples at:https://openslide.cs.cmu.edu/download/openslide-testdata/Hamamatsu/
(I've also verified this with scans produced by JAX microscopy core.)
For a svs (native 256, 256, 3)
chunks='1 MiB'yields up as 512, 512, 3 and is nice and performant.The ndpis end up as either chunksize=(32, 8192, 3) or chunksize=(24, 11520, 3) and are also much improved.
Nota bene: dask by default aims for 128 MiB chunks -- we may consider making this configurable in napari: Preferences: consider adding dask config array.chunk-size along side the cache size napari#7924 -- which is not great for visualization, hence specifying 1 MiB
Note: With this PR for the big ndpi i get more similar, though still slightly worse performance than with QuPath. There is a stutter at the resolution levels (without ASYNC) and with ASYNC it takes a split second to load the 0 level.
If I manually set the chunks, e.g.
chunks=(2048, 2048, 3))orchunks=(1024, 1024, 3))then performance is bit better for the npdi and with ASYNC pretty much identical to QuPath.But I think the auto-chunks are fine for the general use case.