Additional axes bounds as semi-open interval (#7547)

* WIP: Additional axes bounds as semi-open interval * make upper bound exclusive in more places * WIP: migration script for datasource-properties.jsons * naming in nml * fix mag parsing in zarr volume upload * migration * changelog+migration guide * nml snapshots * Update CHANGELOG.unreleased.md Co-authored-by: frcroth <[email protected]> * implement feedback on python migration --------- Co-authored-by: frcroth <[email protected]>
scalableminds · Jan 17, 2024 · e806b44 · e806b44
1 parent 8c1858d
commit e806b44
Show file tree

Hide file tree

Showing 12 changed files with 74 additions and 20 deletions.
diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md
@@ -31,7 +31,8 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
 - Updated antd UI library from version 4.24.8 to 4.24.15. [#7505](https://github.com/scalableminds/webknossos/pull/7505)
 - Changed the default dataset search mode to also search in subfolders. [#7539](https://github.com/scalableminds/webknossos/pull/7539)
 - When clicking a segment in the viewport, it is automatically focused in the segment list. A corresponding context menu entry was added as well. [#7512](https://github.com/scalableminds/webknossos/pull/7512)
-- Updated the isValidName route in the API to return 200 for valid and invalid names. With this, the API version was bumped up to 6. [#7550](https://github.com/scalableminds/webknossos/pull/7550) 
+- Updated the isValidName route in the API to return 200 for valid and invalid names. With this, the API version was bumped up to 6. [#7550](https://github.com/scalableminds/webknossos/pull/7550)
+- The metadata for ND datasets and their annotation has changed: upper bound of additionalAxes is now stored as an exclusive value, called "end" in the NML format. [#7547](https://github.com/scalableminds/webknossos/pull/7547)
 
 ### Fixed
 - Datasets with annotations can now be deleted. The concerning annotations can no longer be viewed but still be downloaded. [#7429](https://github.com/scalableminds/webknossos/pull/7429)
@@ -49,6 +50,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
 - Fixed regression in proofreading tool when automatic mesh loading was disabled and a merge/split operation was performed. [#7534](https://github.com/scalableminds/webknossos/pull/7534)
 - Fixed that last dimension value in ND dataset was not loaded. [#7535](https://github.com/scalableminds/webknossos/pull/7535)
 - Fixed the initialization of the mapping list for agglomerate views if json mappings are present. [#7537](https://github.com/scalableminds/webknossos/pull/7537)
+- Fixed a bug where uploading ND volume annotations would lead to errors due to parsing of the chunk paths. [#7547](https://github.com/scalableminds/webknossos/pull/7547)
 
 ### Removed
 - Removed several unused frontend libraries. [#7521](https://github.com/scalableminds/webknossos/pull/7521)

diff --git a/MIGRATIONS.unreleased.md b/MIGRATIONS.unreleased.md
@@ -12,6 +12,7 @@ User-facing changes are documented in the [changelog](CHANGELOG.released.md).
 - If your setup contains webknossos-workers,  postgres evolution 110 introduces the columns `maxParallelHighPriorityJobs` and `maxParallelLowPriorityJobs`. Make sure to set those values to match what you want for your deployment. [#7463](https://github.com/scalableminds/webknossos/pull/7463)
 - If your setup contains webknossos-workers, you may want to add the new available worker job `compute_segment_index_file` to the `supportedJobCommands` column of one or more of your workers. [#7493](https://github.com/scalableminds/webknossos/pull/7493)
 - The WEBKNOSSOS api version has changed to 6. The `isValidNewName` route for datasets now returns 200 regardless of whether the name is valid or not. The body contains a JSON object with the key "isValid". [#7550](https://github.com/scalableminds/webknossos/pull/7550)
+- If your setup contains ND datasets, run the python3 script at `tools/migrate-axis-bounds/migration.py` on your datastores to update the datasource-properties.jsons of the ND datasets.
 
 ### Postgres Evolutions:
 

diff --git a/app/models/annotation/nml/NmlParser.scala b/app/models/annotation/nml/NmlParser.scala
@@ -270,11 +270,11 @@ object NmlParser extends LazyLogging with ProtoGeometryImplicits with ColorGener
             name <- getSingleAttributeOpt(additionalAxisNode, "name")
             indexStr <- getSingleAttributeOpt(additionalAxisNode, "index")
             index <- indexStr.toIntOpt
-            minStr <- getSingleAttributeOpt(additionalAxisNode, "min")
-            min <- minStr.toIntOpt
-            maxStr <- getSingleAttributeOpt(additionalAxisNode, "max")
-            max <- maxStr.toIntOpt
-          } yield new AdditionalAxisProto(name, index, Vec2IntProto(min, max))
+            startStr <- getSingleAttributeOpt(additionalAxisNode, "start")
+            start <- startStr.toIntOpt
+            endStr <- getSingleAttributeOpt(additionalAxisNode, "end")
+            end <- endStr.toIntOpt
+          } yield new AdditionalAxisProto(name, index, Vec2IntProto(start, end))
         }
       )
     )

diff --git a/app/models/annotation/nml/NmlWriter.scala b/app/models/annotation/nml/NmlWriter.scala
@@ -234,8 +234,8 @@ class NmlWriter @Inject()(implicit ec: ExecutionContext) extends FoxImplicits {
             Xml.withinElementSync("additionalAxis") {
               writer.writeAttribute("name", a.name)
               writer.writeAttribute("index", a.index.toString)
-              writer.writeAttribute("min", a.bounds.x.toString)
-              writer.writeAttribute("max", a.bounds.y.toString)
+              writer.writeAttribute("start", a.bounds.x.toString)
+              writer.writeAttribute("end", a.bounds.y.toString)
             }
           })
         }

diff --git a/frontend/javascripts/oxalis/model/bucket_data_handling/data_cube.ts b/frontend/javascripts/oxalis/model/bucket_data_handling/data_cube.ts
@@ -232,7 +232,7 @@ class DataCube {
       for (const coord of coords || []) {
         if (coord.name in this.additionalAxes) {
           const { bounds } = this.additionalAxes[coord.name];
-          if (coord.value < bounds[0] || coord.value > bounds[1]) {
+          if (coord.value < bounds[0] || coord.value >= bounds[1]) {
             return null;
           }
         }

diff --git a/frontend/javascripts/oxalis/model/helpers/nml_helpers.ts b/frontend/javascripts/oxalis/model/helpers/nml_helpers.ts
@@ -273,14 +273,14 @@ function serializeParameters(
 
         ...(additionalAxes.length > 0
           ? serializeTagWithChildren(
-              "additionalCoordinates",
+              "additionalAxes",
               {},
               additionalAxes.map((coord) =>
-                serializeTag("additionalCoordinate", {
+                serializeTag("additionalAxis", {
                   name: coord.name,
                   index: coord.index,
-                  min: coord.bounds[0],
-                  max: coord.bounds[1],
+                  start: coord.bounds[0],
+                  end: coord.bounds[1],
                 }),
               ),
             )

diff --git a/frontend/javascripts/oxalis/view/action_bar_view.tsx b/frontend/javascripts/oxalis/view/action_bar_view.tsx
@@ -110,7 +110,7 @@ function AdditionalCoordinatesInputView() {
                 label={coord.name}
                 key={coord.name}
                 min={bounds[0]}
-                max={bounds[1]}
+                max={bounds[1] - 1}
                 value={coord.value}
                 spans={[2, 18, 4]}
                 onChange={(newCoord) => {

diff --git a/...end/javascripts/test/snapshots/public-test/test-bundle/test/libs/nml.spec.js.md b/...end/javascripts/test/snapshots/public-test/test-bundle/test/libs/nml.spec.js.md
@@ -82,9 +82,9 @@ Generated by [AVA](https://avajs.dev).
         <zoomLevel zoom="1.3" />␊
         <userBoundingBox topLeftX="5" topLeftY="5" topLeftZ="5" width="245" height="245" depth="245" color.r="1" color.g="0" color.b="0" color.a="1" id="10" name="Test Bounding Box" isVisible="true" />␊
         <taskBoundingBox topLeftX="0" topLeftY="0" topLeftZ="0" width="500" height="500" depth="500" />␊
-        <additionalCoordinates>␊
-          <additionalCoordinate name="t" index="0" min="0" max="100" />␊
-        </additionalCoordinates>␊
+        <additionalAxes>␊
+          <additionalAxis name="t" index="0" start="0" end="100" />␊
+        </additionalAxes>␊
       </parameters>␊
       <thing id="1" color.r="0.09019607843137255" color.g="0.09019607843137255" color.b="0.09019607843137255" color.a="1" name="TestTree-0" groupId="3" type="DEFAULT">␊
         <nodes>␊

diff --git a/frontend/javascripts/test/snapshots/public-test/test-bundle/test/libs/nml.spec.js.snap b/frontend/javascripts/test/snapshots/public-test/test-bundle/test/libs/nml.spec.js.snap
diff --git a/tools/migrate-axis-bounds/migration.py b/tools/migrate-axis-bounds/migration.py
@@ -0,0 +1,51 @@
+from pathlib import Path
+import json
+import shutil
+import time
+import argparse
+
+# Traverses a binaryData directory and changes all datasource-properties.jsons
+# that include additionalAxes, increasing the upper bound by 1
+# This follows a change in the upper bound semantic to be exclusive
+
+def main():
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
+        "--dry",
+        action="store_true",
+    )
+    parser.add_argument("binary_data_dir", type=Path, help="WEBKNOSSOS binary data dir")
+    args = parser.parse_args()
+    dry = args.dry
+    binary_data_dir = args.binary_data_dir
+
+    count = 0
+    print(f"Traversing datasets at {binary_data_dir.resolve()} ...")
+    for orga_dir in [item for item in binary_data_dir.iterdir() if item.is_dir()]:
+        for dataset_dir in [item for item in orga_dir.iterdir() if item.is_dir()]:
+            json_path = dataset_dir / "datasource-properties.json"
+            if json_path.exists():
+                changed = False
+                with open(json_path, 'r') as json_file:
+                    content = json.load(json_file)
+                    for layer in content.get("dataLayers", []):
+                        for axis in layer.get("additionalAxes", []):
+                            if "bounds" in axis:
+                                bounds = axis["bounds"]
+                                if len(bounds) >= 2:
+                                    bounds[1] = bounds[1] + 1
+                                    changed = True
+                if changed:
+                    print(f"Updating {json_path} (dry={dry})...")
+                    count += 1
+                    backup_path = Path(f"{json_path}.{round(time.time() * 1000)}.bak")
+
+                    if not dry:
+                        shutil.copyfile(json_path, backup_path)
+                        with open(json_path, 'w') as json_outfile:
+                            json.dump(content, json_outfile, indent=4)
+
+    print(f"Updated {count} datasets (dry={dry})")
+
+if __name__ == '__main__':
+    main()
diff --git a/webknossos-datastore/app/com/scalableminds/webknossos/datastore/explore/NgffExplorer.scala b/webknossos-datastore/app/com/scalableminds/webknossos/datastore/explore/NgffExplorer.scala
@@ -130,7 +130,7 @@ class NgffExplorer(implicit val ec: ExecutionContext) extends RemoteLayerExplore
           .filter(axis => !defaultAxes.contains(axis.name))
           .zipWithIndex
           .map(axisAndIndex =>
-            createAdditionalAxis(axisAndIndex._1.name, axisAndIndex._2, Array(0, shape(axisAndIndex._2) - 1)).toFox))
+            createAdditionalAxis(axisAndIndex._1.name, axisAndIndex._2, Array(0, shape(axisAndIndex._2))).toFox))
       duplicateNames = axes.map(_.name).diff(axes.map(_.name).distinct).distinct
       _ <- Fox.bool2Fox(duplicateNames.isEmpty) ?~> s"Additional axes names (${duplicateNames.mkString("", ", ", "")}) are not unique."
     } yield axes

diff --git a/...e/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeDataZipHelper.scala b/...e/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeDataZipHelper.scala
@@ -85,7 +85,7 @@ trait VolumeDataZipHelper extends WKWDataFormatHelper with ByteUtils with BoxImp
     val additionalAxesNames: Seq[String] = dimensionNames.toSeq.drop(1).dropRight(3) // drop channel left, and xyz right
 
     // assume additionalAxes,x,y,z
-    val chunkPathRegex = s"(|.*/)(\\d+-\\d+-\\d+)/c\\.(.+)".r
+    val chunkPathRegex = s"(|.*/)(\\d+|\\d+-\\d+-\\d+)/c\\.(.+)".r
 
     path match {
       case chunkPathRegex(_, magStr, dimsStr) =>
@@ -99,7 +99,7 @@ trait VolumeDataZipHelper extends WKWDataFormatHelper with ByteUtils with BoxImp
         val bucketY = dims(dims.length - 2)
         val bucketZ = dims.last
 
-        Vec3Int.fromMagLiteral(magStr).map { mag =>
+        Vec3Int.fromMagLiteral(magStr, allowScalar = true).map { mag =>
           BucketPosition(
             bucketX.toInt * mag.x * DataLayer.bucketLength,
             bucketY.toInt * mag.y * DataLayer.bucketLength,