Various fixes for composing datasets with landmarks #7992

philippotto · 2024-08-14T12:33:21Z

fixes that correspondence points weren't sorted by id during import (sometimes exported NMLs have a weird ordering)
fixes that the TPS was loaded inverted when opening a dataset
prepares for TPS via UI (not activated for now due to Make thin plate splines compatible with datasets that have a scale other than (1,1,1) #7624)
automatically sets the dataset description for composed datasets (links to the used datasets and if an affine transformation was used, the mean error is encoded)

URL of deployed dev instance (used for testing):

https://___.webknossos.xyz

Steps to test:

create NMLs for two datasets and create 4 nodes in each dataset (imagine setting the four corners of a rectangle; should be in the same order (e.g., start top-left and then clock-wise))
download the NMLs
use the compose-dataset tab in the add-dataset screen
provide the two nmls
the newly composed dataset should be created according to the landmarks. the description should be meaningful

Issues:

no issue

(Please delete unneeded items, merge only when none are left open)

Updated changelog

…mments

…escription after composing

…pose-tps

philippotto · 2024-08-19T11:19:03Z

frontend/javascripts/oxalis/model/accessors/dataset_accessor.ts

@@ -722,7 +722,7 @@ function _getOriginalTransformsForLayerOrNull(
  } else if (type === "thin_plate_spline") {
    const { source, target } = transformation.correspondences;

-    return createThinPlateSplineTransform(target, source, dataset.dataSource.scale.factor);
+    return createThinPlateSplineTransform(source, target, dataset.dataSource.scale.factor);


this was an error before. since we don't have any tps in production, there shouldnt be any problems.

MichaelBuessemeyer

The code looks good to me but sadly I found something during testing locally:

When I used the following nmls I could not submit the dataset creation (using affine transformations) and the console showed the following error:

Here are the nmls I used:
l4_first_data_types_second.zip
The nml & dataset order was l4_sample first and then data_types.

Creating a tps dataset with the exact same setup works but showing the dataset crashes due to the same error

MichaelBuessemeyer · 2024-08-19T16:10:43Z

Ok with the help of @valentin-pinkau I was able to identify that all nodes must not be present in the same z slice. This fixes the matrix calculation (the error above).

But I am still unable to see anything with the test setup described above. But that might be due to using two completely different datasets 🙈

MichaelBuessemeyer · 2024-08-20T08:10:38Z

But I am still unable to see anything with the test setup described above. But that might be due to using two completely different datasets 🙈

For the record: Turns out, that the bbox of the dataset is the union of both layers and thus the default view starts at the default center of the "too large" bounding box resulting in not showing any data initially. Using the find my data button solved the problem :)

…rt >= landmarks; test tps creation eagerly etc

philippotto · 2024-08-20T15:16:20Z

@MichaelBuessemeyer please have a look at my latest commit :)

…pose-tps

MichaelBuessemeyer

I only found minor things but also identified, that pushing a copy of all points in the z direction does not cover all cases. Feel free to do this in a follow up pr :)

Will retest now

frontend/javascripts/admin/dataset/composition_wizard/04_configure_new_dataset.tsx

MichaelBuessemeyer · 2024-08-21T14:27:38Z

frontend/javascripts/admin/dataset/composition_wizard/04_configure_new_dataset.tsx

+      const tryAugmentation = await new Promise((resolve) => {
+        Modal.confirm({
+          title: "Augment landmarks?",
+          content:
+            "The provided landmarks can't be used for affine estimation, possibly " +
+            "due to their planar nature. Should a constant translation in the Z " +
+            "direction be assumed, and the landmarks adjusted accordingly?",
+          onOk: () => resolve(true),
+          onCancel: () => resolve(false),
+        });
+      });
+      const augmentLandmarks = (points: Vector3[]) =>
+        points.concat(points.map((p) => [p[0], p[1], p[2] + 1]));
+      if (tryAugmentation) {
+        layersWithTransforms = withTransforms(
+          layersWithoutTransforms,
+          augmentLandmarks(sourcePoints),
+          augmentLandmarks(targetPoints),
+        );
+      } else {
+        throw exception;
+      }
+    }


I am afraid, that always using z for "extrusion" won't cover all cases. In case I annotate all my point in the zy viewport without moving in x direction, the x axis value of all points should be equal. Thus, moving the points by z should fix the problem here. In case we want to catch all possible planes, we should calculate the otrhogol vector to the plane and move the nodes in that direction.

But I can also understand that this might be a bit too much for this pr and can be solved in a follow up pr :)

MichaelBuessemeyer

Testing went well :)

Feel free to merge in case you want to do the only z as a direction for extrusion in another pr

…gure_new_dataset.tsx Co-authored-by: MichaelBuessemeyer <[email protected]>

…o compose-tps

philippotto · 2024-08-21T15:07:17Z

Thanks for the review + test!

I only found minor things but also identified, that pushing a copy of all points in the z direction does not cover all cases. Feel free to do this in a follow up pr :)

Yes, I think, this is fine, because the user is asked whether a constant translation in Z should be assumed. if this is not the case, they should decline and fix it themself.

…o compose-tps

* use tps instead of affine in compose * temporarily disable some CI checks * always sort trees when composing datasets with landmarks * add UI checkbox for affine vs tps * fix that p tag cannot contain ul * also sort nodes by id * fix inverted TPS transform creation when loading dataset; add more comments * render dataset description as markdown in dashboard; update dataset description after composing * fix mean error formatting in description * disable TPS checkbox for now * restore ci * update changelog * make optInfoOut writing independent from IS_TESTING * ask user for landmark augmentation when affine estimation fails; assert >= landmarks; test tps creation eagerly etc * move allowThinPlateSplines into wk dev flags * Update frontend/javascripts/admin/dataset/composition_wizard/04_configure_new_dataset.tsx Co-authored-by: MichaelBuessemeyer <[email protected]> * move guardedWithErrorToast fn * format * fix cyclic dependency --------- Co-authored-by: MichaelBuessemeyer <[email protected]>

philippotto added 2 commits August 14, 2024 14:32

use tps instead of affine in compose

10fb4e4

temporarily disable some CI checks

d571e52

philippotto changed the title ~~[WIP] Compose TPS~~ [WIP] Compose datasets with thin plate splines Aug 14, 2024

philippotto added 8 commits August 14, 2024 17:14

always sort trees when composing datasets with landmarks

0f7eebb

add UI checkbox for affine vs tps

509b7db

fix that p tag cannot contain ul

b624b58

also sort nodes by id

ddf1d26

fix inverted TPS transform creation when loading dataset; add more co…

5aae4d9

…mments

render dataset description as markdown in dashboard; update dataset d…

cef63e8

…escription after composing

fix mean error formatting in description

5a690e1

Merge branch 'master' of github.com:scalableminds/webknossos into com…

61b4155

…pose-tps

philippotto commented Aug 19, 2024

View reviewed changes

disable TPS checkbox for now

8fcc8ee

philippotto changed the title ~~[WIP] Compose datasets with thin plate splines~~ Compose datasets with thin plate splines Aug 19, 2024

restore ci

c6227b1

philippotto self-assigned this Aug 19, 2024

philippotto marked this pull request as ready for review August 19, 2024 11:44

philippotto requested a review from MichaelBuessemeyer August 19, 2024 11:44

philippotto changed the title ~~Compose datasets with thin plate splines~~ Various fixes for composing datasets with landmarks Aug 19, 2024

update changelog

7ccf57c

MichaelBuessemeyer requested changes Aug 19, 2024

View reviewed changes

make optInfoOut writing independent from IS_TESTING

e74a069

ask user for landmark augmentation when affine estimation fails; asse…

734d426

…rt >= landmarks; test tps creation eagerly etc

philippotto requested a review from MichaelBuessemeyer August 20, 2024 15:19

philippotto added 2 commits August 21, 2024 08:54

Merge branch 'master' of github.com:scalableminds/webknossos into com…

17f51d8

…pose-tps

move allowThinPlateSplines into wk dev flags

173e093

MichaelBuessemeyer reviewed Aug 21, 2024

View reviewed changes

MichaelBuessemeyer approved these changes Aug 21, 2024

View reviewed changes

philippotto and others added 3 commits August 21, 2024 17:04

Update frontend/javascripts/admin/dataset/composition_wizard/04_confi…

c2a9a70

…gure_new_dataset.tsx Co-authored-by: MichaelBuessemeyer <[email protected]>

move guardedWithErrorToast fn

ff5415b

Merge branch 'compose-tps' of github.com:scalableminds/webknossos int…

0f00869

…o compose-tps

philippotto and others added 4 commits August 21, 2024 17:07

Merge branch 'master' into compose-tps

5da255a

format

6bee6c5

Merge branch 'compose-tps' of github.com:scalableminds/webknossos int…

c977210

…o compose-tps

fix cyclic dependency

4bab1a9

philippotto enabled auto-merge (squash) August 22, 2024 07:26

philippotto merged commit 54a93ee into master Aug 22, 2024
2 checks passed

philippotto deleted the compose-tps branch August 22, 2024 07:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Various fixes for composing datasets with landmarks #7992

Various fixes for composing datasets with landmarks #7992

philippotto commented Aug 14, 2024 •

edited

Loading

philippotto Aug 19, 2024

MichaelBuessemeyer left a comment

MichaelBuessemeyer commented Aug 19, 2024

MichaelBuessemeyer commented Aug 20, 2024

philippotto commented Aug 20, 2024

MichaelBuessemeyer left a comment

MichaelBuessemeyer Aug 21, 2024

MichaelBuessemeyer left a comment

philippotto commented Aug 21, 2024

Various fixes for composing datasets with landmarks #7992

Various fixes for composing datasets with landmarks #7992

Conversation

philippotto commented Aug 14, 2024 • edited Loading

URL of deployed dev instance (used for testing):

Steps to test:

Issues:

philippotto Aug 19, 2024

Choose a reason for hiding this comment

MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

MichaelBuessemeyer commented Aug 19, 2024

MichaelBuessemeyer commented Aug 20, 2024

philippotto commented Aug 20, 2024

MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

MichaelBuessemeyer Aug 21, 2024

Choose a reason for hiding this comment

MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

philippotto commented Aug 21, 2024

philippotto commented Aug 14, 2024 •

edited

Loading