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

Zarr3 download for volume annotation, supporting nd #7288

Merged
merged 195 commits into from
Oct 17, 2023
Merged

Conversation

fm3
Copy link
Member

@fm3 fm3 commented Aug 17, 2023

URL of deployed dev instance (used for testing):

Steps to test:

  • Create annotation on 3d dataset, see download modal, try different options
  • Create annotation on nd dataset, see download modal, try different options
  • Download from other places like dashboard should still use wkw for the moment (untill the libs support zarr)
  • Reupload of a zarr volume annotation should yield the same annotation
  • Zarr metadata and datasource-properties.json in data.zip should look right.
  • Unpacking a zarr data.zip and moving it to the a folder in binaryData should yield a usable webknossos dataset (this does not yet work in nd case because only specific axis orders are supported with additional coordinates)

TODOs:

  • Backend
    • add parameter selecting download as zarr
    • write zarr chunks
    • support nd in writing zarr chunks, (what axis order? should channels be included?)
    • write zarr header (how too persist axis order?)
      • Q: should the shape be the full layer shape? (starting at 0,0,0)? A: yes
      • Q: should the fill_value be set? (0 is kind of wrong due to fallback layer?) A: use 0 anyway
      • Q: should we set any additional attributes?
      • Get rid of the auto-generated field for the codecs → I don’t know how :(
    • save compressed buckets
    • save compression info in the zarr.json
    • make parameter enum instead
    • include dataZipFormat as tag in NML
    • support upload
      • in upload, distinguish between volume formats
        • Q: should this be auto-detected from the contents or taken from the NML? Auto-detect for now
      • get bucket position with additional coordinates from paths
      • in upload, include assertions for the right zarr format
      • support nd
      • support merging volumes when uploading multiple
    • include datasource-properties.json in data.zip to enable using this as a normal dataset
    • include channel axis
      • why does wk no longer display the layer as normal dataset? (even in 3d+c case)
    • in zarr.json metadata, use array-of-int transpose codec instead of "F".
  • Frontend
    • Select zarr3 download format for annotations with additional coordinates
    • Say so in the download modal
    • In places where downloads are started without the modal (i.e. dashboard), keep wkw for now until the libs support the zarr-based format, compare Support Zarr3 Volume Annotation format webknossos-libs#951

Issues:


philippotto and others added 30 commits June 12, 2023 14:50
@MichaelBuessemeyer
Copy link
Contributor

I took care of the frontend part of this pr:
The download modal now gives the user the option to choose between wkw and zarr for annotations with volume data. The UI looks like this in the current version:
image
In case one of the volume layers is nd, wkw is disabled as a download format and only zarr is available.

The other cases where annotations are downloaded (like tasks or projects), nd datasets are not supported for download right now.

- enforce annotation download in zarr when having nd datasets
@fm3 fm3 changed the title WIP: zarr3 download for volume annotation, supporting nd Zarr3 download for volume annotation, supporting nd Oct 11, 2023
@fm3 fm3 marked this pull request as ready for review October 11, 2023 12:03
@@ -1035,21 +1035,28 @@ export async function downloadAnnotation(
annotationType: APIAnnotationType,
showVolumeFallbackDownloadWarning: boolean = false,
versions: Versions = {},
volumeDataZipFormat: string = "wkw",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the type could be "zarr3" | "wkw" instead of string, I think.

@@ -268,10 +278,13 @@ function _DownloadModalView({
(state: OxalisState) => state.temporaryConfiguration.isMergerModeEnabled,
);
const hasVolumeFallback = tracing.volumes.some((volume) => volume.fallbackLayer != null);
const isVolumeNDimensional = tracing.volumes.some((tracing) => tracing.additionalAxes.length > 0);
const hasVolumes = hasVolumeTracings(tracing);
const initialFilesToDownload = hasVolumes ? (isVolumeNDimensional ? "zarr" : "wkw") : "nml";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rename initialFilesToDownload and filesToDownload to [initial]fileFormatToDownload. otherwise, it sounds like the vars would contain arrays.

@@ -547,46 +555,42 @@ function _DownloadModalView({
}}
>
Select the data you would like to download.
<Hint style={{ marginTop: 12 }}>
An NML file will always be included with any download.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will the NML always contain the skeletons? if not, then it's not possible to download volume + skeleton at the same time? and if yes, then it's not possible to download volume without skeletons? either way, it seems as this gives less control to the user? is this intended?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, skeletons are always included. This was already the case before this PR. We might want to change that in the future, but that’s unrelated to this change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see, then never mind :)

@fm3 fm3 merged commit ede79a0 into master Oct 17, 2023
2 checks passed
@fm3 fm3 deleted the nd-volume-download branch October 17, 2023 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support download+upload of ND volume annotations
4 participants