Adds OME-TIFF export #6838

normanrz · 2023-02-10T07:59:08Z

Adds OME-Tiff export
Adds mag selection in the export modal
Gets rid of the specialized bbox export modal (in favor of the regular download modal)
The download modal polls for the export job status and immediately begins download, when complete
Cleans up some job parameters in the backend

Steps to test:

Checkout the ome-tiff-export branch of vx
Start the worker
Start wk
Try to export an OME-TIFF
Validate that the exported data is right with Fiji

Updated (unreleased) changelog
Updated (unreleased) migration guide if applicable
Updated documentation if applicable
Needs worker update after deployment
Ready for review

normanrz · 2023-02-10T07:59:39Z

frontend/javascripts/admin/job/job_list_view.tsx

        <div
+          className="clearfix"


I only got rid of this div in this file

philippotto

Great work & clean up 👍 I didn't test it, but left some comments.

frontend/javascripts/oxalis/view/right-border-tabs/starting_job_modals.tsx

frontend/javascripts/oxalis/view/right-border-tabs/bounding_box_tab.tsx

frontend/javascripts/admin/admin_rest_api.ts

philippotto · 2023-02-10T09:28:49Z

frontend/javascripts/oxalis/view/action-bar/download_modal_view.tsx

+  };
+}
+
+function useRunningJobs(): [


Maybe you can have a look at the useStartAndPollJob hook in the job_hooks.ts module to check whether there is some intersection which could be extracted/reused.

philippotto · 2023-02-10T10:09:55Z

frontend/javascripts/oxalis/view/action-bar/download_modal_view.tsx

+  const { lowestResolutionIndex, highestResolutionIndex } = selectedLayerInfos;
+  const [rawResolutionIndex, setResolutionIndex] = useState<number>(lowestResolutionIndex);
+  const resolutionIndex = clamp(lowestResolutionIndex, rawResolutionIndex, highestResolutionIndex);
+  const [exportFormat, setExportFormat] = useState<ExportFormat>(ExportFormat.OME_TIFF);


is ome tiff widely supported and thus a good default?

Yes, I think so. We should probably track usage over time.

frontend/javascripts/oxalis/view/action-bar/download_modal_view.tsx

fm3

Backend LGTM :)
I can do a testing round once the other feedback is adressed

philippotto · 2023-02-16T09:13:06Z

frontend/javascripts/admin/job/job_hooks.ts

-    setJobs(await getJobs());
-  }, interval);
+}): [
+  Array<[string, string]>,


Suggested change

Array<[string, string]>,

Array<[jobKey: string, jobId: string]>,

would make this more readable

philippotto · 2023-02-16T09:19:51Z

frontend/javascripts/admin/job/job_hooks.ts

-    mostRecentSuccessfulJob,
-  };
+  usePolling(checkForJobs, runningJobs.length > 0 ? interval : null, [
+    runningJobs.map(([key]) => key).join("-"),


why are these dependencies necessary? the delay (which depends on runningJobs) is already passed as a dependency within the hook. I would have thought that this should suffice?

philippotto · 2023-02-16T09:57:09Z

frontend/javascripts/dashboard/dataset/dataset_settings_data_tab.tsx

+          "The computation of the largest segment id for this dataset has finished. Please reload the page to see it.",
+        );
+      }
+      setMostRecentSuccessfulJob(job);


how does this ensure that this is the most recent job? the result of it is rendered in line 600 and could be confusing if it shows outdated data.

also, if there have been multiple jobs already, the callbacks will be called multiple times as far as I see. thus, multiple toasts will be shown. I think, adapting the interface of the hook so that the mostRecentSuccessfulJob is emitted somehow would be nicer.

philippotto · 2023-02-16T09:59:29Z

frontend/javascripts/oxalis/view/action-bar/download_modal_view.tsx

+    async onSuccess(job) {
+      if (job.resultLink != null) {
+        const token = await doWithToken(async (t) => t);
+        saveAs(`${job.resultLink}?token=${token}`);
+      }
+    },
+    onFailure() {


shouldn't these handle the isInitial parameter so that these don't fire for old jobs? maybe it would be better to have dedicated callbacks for "past jobs".

ah, this won't be a problem since the initial key fn is not passed to the hook.

fm3

Very cool stuff, the UI works nicely for me!

I had an error in the export job, related to this now being available for coarser mags:

  File "/home/f/scm/code/voxelytics/voxelytics/worker/jobs/export_tiff.py", line 209, in write_tiff_stack
    with input_mag_view.get_buffered_slice_reader(
  File "/home/f/scm/code/voxelytics/environments/miniconda/envs/vx-py39/lib/python3.9/site-packages/webknossos/dataset/view.py", line 683, in get_buffered_slice_reader
    return BufferedSliceReader(
  File "/home/f/scm/code/voxelytics/environments/miniconda/envs/vx-py39/lib/python3.9/site-packages/webknossos/dataset/_utils/buffered_slice_reader.py", line 61, in __init__
    self.bbox_current_mag = absolute_bounding_box.in_mag(view.mag)
  File "/home/f/scm/code/voxelytics/environments/miniconda/envs/vx-py39/lib/python3.9/site-packages/webknossos/geometry/bounding_box.py", line 306, in in_mag
    assert (
AssertionError: topleft Vec3Int(0,31,0) is not aligned with the mag 2. Use BoundingBox.align_with_mag().

Since the user cannot see the error message, maybe this should be caught already before? (Or just pass a mag-aligned bbox to the tiff export code?) This happened only in the tiff stack export, not for ome-tiff.

With a mag-aligned bbox, both exports work fine :)

Another thing: the bbox tool by default places the bbox depending on the current viewport dimensions. This often leads to negative start values when zoomed out. Maybe it would make sense to limit the bboxes (or at least their initial params when hitting the plus sign) to the dataset bbox? Currently, the backend rejects export jobs with such bboxes, saying “The selected bounding box could not be parsed, must be x,y,z,width,height,depth”. So I don’t know what the worker would do with them.

philippotto · 2023-02-23T12:29:09Z

The last commits look good 👍

Since the user cannot see the error message, maybe this should be caught already before? (Or just pass a mag-aligned bbox to the tiff export code?) This happened only in the tiff stack export, not for ome-tiff.

Is this fixed now? Since this seems like an annoying problem with a potentially easy solution, I'd rather fix it now. The frontend has a BoundingBox.alignWithMag method which should be easy to use here.

normanrz · 2023-02-23T12:30:31Z

Is this fixed now? Since this seems like an annoying problem with a potentially easy solution, I'd rather fix it now. The frontend has a BoundingBox.alignWithMag method which should be easy to use here.

I fixed it in the vx worker code. Do you think it should rather be fixed in the frontend?

philippotto · 2023-02-23T12:32:20Z

I fixed it in the vx worker code. Do you think it should rather be fixed in the frontend?

Ah, I didn't know that. That's even better 👍

philippotto · 2023-02-23T12:32:40Z

The new worker should probably be released first before merging this feature, though.

fm3 · 2023-02-23T12:43:47Z

The backend still rejects bboxes with negative topleft, though. Do you think we should adapt the bbox literal parsing in the backend to support negative toplefts? Or rather clamp it in the frontend?

normanrz · 2023-02-23T14:50:15Z

I fixed the regex in the backend, so that negative bboxes are accepted. The worker then intersects the bboxes with the layer-bbox.

…d-v4.24 * 'master' of github.com:scalableminds/webknossos: Implement http range requests for HttpsSeekableByteChannel (#6869) new GH action for adding issues to project board Fix links in Changelog (#6881) adds dedicated explore method for zarr datasets with a datasource-properties.json (#6879) Release 23.03.0 (#6880) Fix superUser being wrongly marked as organization owners (#6876) Followups for OME-TIFF export (#6874) Fix reload-precomputed-mesh functionality (#6875) Adds OME-TIFF export (#6838) Add evolutions 99,100 to migration guide (#6871) Add link to imprint and privacy to help menu (#6870) Annotation Locking Mechanism (#6819)

…come_header_UI * 'master' of github.com:scalableminds/webknossos: (34 commits) Slim down view mode dropdown by using icons (#6900) Logging on password reset/change (#6901) When merging volume tracings, also merge segment lists (#6882) avoid spinner when switching tabs in dashboard (#6894) Upgrade Antd to v4.24 (#6865) Support n5 end-chunks with chunksize differing from metadata chunksize (#6890) Implement http range requests for HttpsSeekableByteChannel (#6869) new GH action for adding issues to project board Fix links in Changelog (#6881) adds dedicated explore method for zarr datasets with a datasource-properties.json (#6879) Release 23.03.0 (#6880) Fix superUser being wrongly marked as organization owners (#6876) Followups for OME-TIFF export (#6874) Fix reload-precomputed-mesh functionality (#6875) Adds OME-TIFF export (#6838) Add evolutions 99,100 to migration guide (#6871) Add link to imprint and privacy to help menu (#6870) Annotation Locking Mechanism (#6819) Update deprecated antd <Menu> (#6860) Add functions to get and set segment colors to the frontend API (#6853) ...

normanrz added 9 commits February 6, 2023 15:05

adds OME-TIFF export support

a662f87

Merge remote-tracking branch 'origin/master' into ome-tiff-export

d4ac559

stuff

df586a1

wip

e7be125

Merge remote-tracking branch 'origin/master' into ome-tiff-export

e58e637

stuff

2d40104

Merge remote-tracking branch 'origin/master' into ome-tiff-export

3f519e2

styling

3b93e5f

lint

af68170

normanrz requested review from fm3 and philippotto February 10, 2023 07:59

normanrz self-assigned this Feb 10, 2023

normanrz commented Feb 10, 2023

View reviewed changes

frontend/javascripts/admin/job/job_list_view.tsx

<div

className="clearfix"

Copy link

Member Author

normanrz Feb 10, 2023

Choose a reason for hiding this comment

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

I only got rid of this div in this file

philippotto requested changes Feb 10, 2023

View reviewed changes

fm3 reviewed Feb 13, 2023

View reviewed changes

normanrz added 5 commits February 14, 2023 09:58

pr feedback

cca4b2e

wip

0fee156

rewrite job-polling hooks

287fca4

merge

80364e6

docs + changelog + migration guide

6caa9e8

normanrz requested a review from philippotto February 14, 2023 16:06

philippotto reviewed Feb 16, 2023

View reviewed changes

fm3 reviewed Feb 16, 2023

View reviewed changes

normanrz added 3 commits February 20, 2023 10:04

Merge remote-tracking branch 'origin/master' into ome-tiff-export

0e0df47

pr feedback

75b0e2f

reverse sort order

6d38236

normanrz requested a review from philippotto February 20, 2023 14:20

non-capturing group

40b42f2

fm3 approved these changes Feb 23, 2023

View reviewed changes

philippotto approved these changes Feb 23, 2023

View reviewed changes

normanrz merged commit ef65a5c into master Feb 24, 2023

normanrz deleted the ome-tiff-export branch February 24, 2023 08:35

fm3 mentioned this pull request Mar 9, 2023

Tiff export for layers that do not have mag1 #5260

Closed

philippotto mentioned this pull request Mar 20, 2023

Fix download button for annotations when tiff export is disabled #6931

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds OME-TIFF export #6838

Adds OME-TIFF export #6838

normanrz commented Feb 10, 2023 •

edited

Loading

normanrz Feb 10, 2023

philippotto left a comment

philippotto Feb 10, 2023

philippotto Feb 10, 2023

normanrz Feb 10, 2023

fm3 left a comment

philippotto Feb 16, 2023

philippotto Feb 16, 2023

philippotto Feb 16, 2023

philippotto Feb 16, 2023

philippotto Feb 16, 2023

fm3 left a comment

philippotto commented Feb 23, 2023

normanrz commented Feb 23, 2023

philippotto commented Feb 23, 2023

philippotto commented Feb 23, 2023

fm3 commented Feb 23, 2023

normanrz commented Feb 23, 2023

	Array<[string, string]>,
	Array<[jobKey: string, jobId: string]>,

Adds OME-TIFF export #6838

Adds OME-TIFF export #6838

Conversation

normanrz commented Feb 10, 2023 • edited Loading

Steps to test:

Choose a reason for hiding this comment

philippotto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fm3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fm3 left a comment

Choose a reason for hiding this comment

philippotto commented Feb 23, 2023

normanrz commented Feb 23, 2023

philippotto commented Feb 23, 2023

philippotto commented Feb 23, 2023

fm3 commented Feb 23, 2023

normanrz commented Feb 23, 2023

normanrz commented Feb 10, 2023 •

edited

Loading