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

Multi-Resolution Volume Tracings #4755

Merged
merged 157 commits into from
Nov 9, 2020
Merged

Multi-Resolution Volume Tracings #4755

merged 157 commits into from
Nov 9, 2020

Conversation

fm3
Copy link
Member

@fm3 fm3 commented Aug 6, 2020

Introducing Volume Annotations in Multiple Resolutions.

  • Volume tracing objects now contain a list of resolutions in which buckets should be created and saved.
  • The front-end simultaneously updates buckets in all of those resolutions whenever the user changes the segmentation.
  • Every brush stroke / fill action etc. is up/downsampled to all those resolutions.
  • The back-end volume adds explicit downsampling functionality so that legacy volume annotations can be converted in this way.
  • The list of resolutions is determined by the fallback layer (use its resolutions if it is present) or the union of all layers (use these resolutions if there is no fallback layer).
  • When merging multiple volume annotations, the resolution sets have to be identical.
  • Task Types can restrict resolutions. If a volume/hybrid task has resolution restrictions, only those resolutions are present in the task volumes.

Related design doc in notion (some points may be outdated, see comments here)

Note that a secondary PR #4891 has been merged into this one, overwriting some of the discussions AND steps-to-test here.

URL of deployed dev instance (used for testing):

TODO:

Steps to test:

( Note that a secondary PR #4891 has been merged into this one, overwriting some of the discussions AND steps-to-test here. )

Back-end focussing tests

  • open legacy volume annotation, brushing + filling + tracing should still work and save mag1-buckets
  • open legacy volume annotation, zooming out should display warning about invisible segmentation
  • create new volume annotation with fallback layer, tracing should have mags of fallback layer
  • create new volume annotation without fallback layer, tracing should have all mags (union) of all layers
  • upload legacy volume annotation via dashboard, resulting tracing should have the specified mag list, and have its data downsampled accordingly
  • create new volume annotation with fallback layer, zooming out to resolution where fallbacklayer/volume data is not present should display warning about invisible segmentation
  • create new volume annotation with fallback layer only present in mag2 (but color data in all mags), volume annotation should have mag2 only (and create mag2 data only)
  • create new volume annotation, brush some, download (data.zip should have bucket data in each mag)
  • reupload this multi-resolution volume annotation, should have same resolution list and volume data
  • open legacy or new volume annotation, click »unlink fallback segmentation«, should reload with new empty volume tracing that has all mags (union) of all present dataset layers
  • create new hybrid annotation, there should not be differences from volume-only concerning the volume data
  • check that both upload/download and brushing work with isotropically and anisotropically downsampled datasets

Front-end focussing tests

  • test with volume tracing without fallback layer
    • test brush
      • brush in mag 1, zoom out and check that this is also visible in higher mags
      • brush in mag 2-2-1, zoom in and check that this is also visible in mag1. note that multiple slices should be annotated.
    • "polygon"-trace
      • "polygon"-trace in mag 1, zoom out and check that this is also visible in higher mags
      • "polygon"-trace in mag 2-2-1, zoom in and check that this is also visible in mag1. note that multiple slices should be annotated.
    • use "copy segmentation from last slice"-tool (V shortcut)
      • copy segmentation in mag 1, zoom out and check that this is also visible in higher mags
      • copy segmentation in mag 2-2-1, zoom in and check that this is also visible in mag1. note that multiple slices should be annotated.
        • when zooming between mags, copy-segmentation sometimes does not work? double check why.
  • test with volume tracing with fallback layer (which has the same mags as the color layer)
    • test brush
      • brush in mag 1, zoom out and check that this is also visible in higher mags
      • brush in mag 2-2-1, zoom in and check that this is also visible in mag1. note that multiple slices should be annotated.
    • "polygon"-trace
      • "polygon"-trace in mag 1, zoom out and check that this is also visible in higher mags
      • "polygon"-trace in mag 2-2-1, zoom in and check that this is also visible in mag1. note that multiple slices should be annotated.
    • use "copy segmentation from last slice"-tool (V shortcut)
      • copy segmentation in mag 1, zoom out and check that this is also visible in higher mags
      • copy segmentation in mag 2-2-1, zoom in and check that this is also visible in mag1. note that multiple slices should be annotated.
  • test with volume tracing with fallback layer (which does not have mag1)
    • ensure that "render missing data" black option is disabled
    • test brush
      • brush in mag 1, zoom out and check that this is also visible in higher mags
      • brush in mag 2-2-1, zoom in and check that this is also visible in mag1. note that multiple slices should be annotated.
    • "polygon"-trace
      • "polygon"-trace in mag 1, zoom out and check that this is also visible in higher mags
      • "polygon"-trace in mag 2-2-1, zoom in and check that this is also visible in mag1. note that multiple slices should be annotated.
    • use "copy segmentation from last slice"-tool (V shortcut)
      • copy segmentation in mag 1, zoom out and check that this is also visible in higher mags
      • copy segmentation in mag 2-2-1, zoom in and check that this is also visible in mag1. note that multiple slices should be annotated.
  • test with volume tracing with fallback layer (which has "holes" in the mag list; e.g., mag 1 and mag 4 exists, but mag 2 does not)
    • ensure that an warning is shown in the toolbar when trying to annotate something --> volume annotation should be disabled in the missing mags (if it can't be rendered)

Issues:


@MichaelBuessemeyer
Copy link
Contributor

MichaelBuessemeyer commented Aug 12, 2020

TODOs frontend:
And they should be used by the frontend correctly.

  • Disable the auto-fill feature for the brush if e.g.
    • the magnification is too high
    • The brushed area is too big
  • When the trace tool is disabled: just do it in grey, not and "aggressive" orange
  • Check whether this works with copy to layer
  • Check whether floodfill works
  • Apply upscaling to as many slices as needed
  • fix frontend CI tests
  • handle tracings where resolution pyramid is not full. additionally, showing the “zoom in to view segmentation” toast again, if the segmentation cannot be rendered.
  • frontend should use the unlinkFallback route, adding an explaining mouseover text that the volume annotation is lost. this is not versioned and cannot be undone (add confirmation dialogue?)
  • add frontend-specific steps to test to the list in PR description above
  • adapt api so that labelVoxels works for automatically for all resolutions
  • ensure merger mode works in higher mags
  • don't try to fetch data from missing mags
  • fix "Render Missing Data Black" works again

For the rest, see #4838.

@philippotto philippotto self-assigned this Oct 23, 2020
@philippotto
Copy link
Member

I just got the following back-end exception:

play.api.http.HttpErrorHandlerExceptions$$anon$1: Execution exception[[ArithmeticException: / by zero]]
        at play.api.http.HttpErrorHandlerExceptions$.throwableToUsefulException(HttpErrorHandler.scala:351)
        at play.api.http.DefaultHttpErrorHandler.onServerError(HttpErrorHandler.scala:267)
        at play.core.server.AkkaHttpServer$$anonfun$1.applyOrElse(AkkaHttpServer.scala:448)
        at play.core.server.AkkaHttpServer$$anonfun$1.applyOrElse(AkkaHttpServer.scala:446)
        at scala.concurrent.Future.$anonfun$recoverWith$1(Future.scala:413)
        at scala.concurrent.impl.Promise.$anonfun$transformWith$1(Promise.scala:37)
        at scala.concurrent.impl.CallbackRunnable.run(Promise.scala:60)
        at akka.dispatch.BatchingExecutor$AbstractBatch.processBatch(BatchingExecutor.scala:55)
        at akka.dispatch.BatchingExecutor$BlockableBatch.$anonfun$run$1(BatchingExecutor.scala:92)
        at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:12)
Caused by: java.lang.ArithmeticException: / by zero
        at com.scalableminds.webknossos.datastore.models.BucketPosition.<init>(Positions.scala:52)
        at com.scalableminds.webknossos.tracingstore.tracings.volume.VolumeTracingService.$anonfun$handleUpdateGroup$2(VolumeTracingService.scala:94)
        at scala.concurrent.Future.$anonfun$flatMap$1(Future.scala:303)
        at scala.concurrent.impl.Promise.$anonfun$transformWith$1(Promise.scala:37)
        at scala.concurrent.impl.CallbackRunnable.run(Promise.scala:60)
        at java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1402)
        at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)
        at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)
        at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)
        at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:175)

I opened a hybrid tracing with fallback segmentation and brushed a bit. Not sure yet, what exactly this is causing. Do you have a spontaneous idea, @fm3, or should I investigate?

@philippotto
Copy link
Member

Here's a screenshot of the save queue content:

image

@fm3
Copy link
Member Author

fm3 commented Oct 23, 2020

I think there may be a mismatch between the zoomstep and the tracing’s resolution list.
Can you reproduce it?
Could you replace VolumeTracingService.scala’s line 131 with } else throw new Exception(s"Received bucket with zoomStep ($zoomStep), could not look up that resolution tracing.resolutions (${tracing.resolutions})")
(I suppose we should do that anyway)

Also, just to make sure: is the zoomStep’s semantic that it is the index in the (sorted) tracing resolutions list? or should we check zoomStep**2 against maxDim?

@philippotto
Copy link
Member

Could you replace VolumeTracingService.scala’s line 131 with } else throw new Exception(s"Received bucket with zoomStep ($zoomStep), could not look up that resolution tracing.resolutions (${tracing.resolutions})")
(I suppose we should do that anyway)

Yes, this seems like a good solution! The error was raised:

Exception: Received bucket with zoomStep (3), could not look up that resolution tracing.resolutions (Vector(Point3D(2,2,2), Point3D(8,8,8)))

Also, just to make sure: is the zoomStep’s semantic that it is the index in the (sorted) tracing resolutions list? or should we check zoomStep**2 against maxDim?

Yes, this seems to be the problem. Before this PR, the zoomStep was always the index AND the exponent for the power-of-two-mag sequence. So, it didn't really matter. With the changes of this PR, the zoomStep should be interpreted as the exponent, so that the power-of-two is used to find the correct resolution (using maxDim). So, in this case zoomStep == 3 should map to [8, 8, 8]. Can you change this? I'll commit the fix you mentioned in the meantime :)

@fm3
Copy link
Member Author

fm3 commented Oct 23, 2020

I changed that. I think this was the only volume tracing related spot where this was done as index lookup. Data loading already did it with the power-of-two matching. Good catch!

@philippotto
Copy link
Member

Awesome! Works perfectly now :) I think, I also worked out the last kinks which were caused by the new toolbar.

philippotto and others added 4 commits November 3, 2020 14:17
…4891)

* [WIP] enforce task type magnification settings in tasks

* pass allowedMagnifications to rpc methods

* pass magnifications as query string

* respect resolution restrictions for volume tracings.

* add option to downsample existing volume tracing

* remove outdated assertion, update resolution list after downsampling

* sleep 10s

* remove sleep and bump dev-proxy-timeout to 5 min

* fix task creation with resolution restrictions

* use inclusive check

* when uploading zips with differing resolution sets, fail

* remove unused import

* refresh snapshots (tracings contain organization name)

* write header.wkw to every mag in volume download, fix anisotropic directory names

* add resolution-samenss assertion in volume dnd import. fix flow types

* fix restriction passing in initial data

* Add button to trigger downsampling of volume annotation.

The reload button next to the active-resolution-indicator only appears
for explorative annotations with volume data. It opens a modal which
explains the action and its consequences. While the operation is
running, the modal blocks further usages of webKnossos. After the
operation has completed, the complete page is reloaded.

* fix parameter passing in duplicate rpc

* avoid undefined behavior of bucket iterator by checking hasNext. do not relabel when “merging” only one volume

* make tasktype resolution restrictions immutable. sort resolution list

* update migration guide

* remove debug output

* improve initiate-volume-downsampling modal and move its button to the layer settings

* show toast when mag-restriction is violated instead of prohibiting the zoom change in the first place

* don't allow editing mag-restrictions in already created tasktype

* remove some more debug output, unify variable names

* forbid creating new nodes or using volume tools when the current mag was forbidden by the task type

* clean up

* Update MIGRATIONS.unreleased.md

Co-authored-by: Philipp Otto <[email protected]>

* fix styling of button-link

* use ListBuffer instead of HashSet as per pr feedback

* integrate some feedback

* Apply suggestions from code review

Co-authored-by: Daniel <[email protected]>

* remove unnecessary SET_TOOL dependence

* do map ids if initialLargestSegmentId != 0

Co-authored-by: Philipp Otto <[email protected]>
Co-authored-by: Philipp Otto <[email protected]>
Co-authored-by: Daniel <[email protected]>
Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

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

Let's roll 🎡

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.

Available resolutions list per layer Handle multiple volume resolutions for hybrid annotations
6 participants