Skip to content

Support OME-NGFF zarr dataset translation #8311

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.

Sign up for GitHub

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

Merged
merged 12 commits into from
Jan 15, 2025
Merged

Support OME-NGFF zarr dataset translation #8311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
9a8871c
Support 'units' key
frcroth Jan 6, 2025
5bd3ec4
Ignore label layers for now
frcroth Jan 6, 2025
58620ec
Support ngff coordinateTransformations translation property
frcroth Jan 8, 2025
eb7af06
Update changelog
frcroth Jan 8, 2025
a1c816d
Merge branch 'master' into suport-ngff-translate
frcroth Jan 8, 2025
adba7f8
Lint
frcroth Jan 8, 2025
78301d7
Revert "Support 'units' key"
frcroth Jan 8, 2025
bbdde25
Use headOption instead of head
frcroth Jan 13, 2025
a4700cd
Combine generated ct with existing ct
frcroth Jan 13, 2025
52fc186
Merge branch 'master' into suport-ngff-translate
frcroth Jan 13, 2025
ba5b24c
Merge branch 'master' into suport-ngff-translate
frcroth Jan 15, 2025
851c88a
Readd label layer exploration
frcroth Jan 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.unreleased.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
- Added the option for "Selective Segment Visibility" for segmentation layers. Select this option in the left sidebar to only show segments that are currently active or hovered. [#8281](https://github.com/scalableminds/webknossos/pull/8281)
- A segment can be activated with doubleclick now. [#8281](https://github.com/scalableminds/webknossos/pull/8281)
- It is now possible to select the magnification of the layers on which an AI model will be trained. [#8266](https://github.com/scalableminds/webknossos/pull/8266)
- Added support for translations in OME NGFF zarr datasets (translation within coordinateTransformations on datasets). [#8311](https://github.com/scalableminds/webknossos/pull/8311)
- When the eraser tool is active, one can switch temporarily to the fill-segment tool by pressing shift and ctrl. Only pressing shift, switches to the pick-segment tool. [#8314](https://github.com/scalableminds/webknossos/pull/8314)
- Enabled auto sorting of Typescript imports in Biome linter. [#8313](https://github.com/scalableminds/webknossos/pull/8313)

Expand Down
7 changes: 5 additions & 2 deletions ...-datastore/app/com/scalableminds/webknossos/datastore/datareaders/zarr/NgffMetadata.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,11 @@ object NgffMetadata {
mag =>
NgffDataset(
path = mag.toMagLiteral(allowScalar = true),
List(NgffCoordinateTransformation(
scale = Some(List[Double](1.0) ++ (dataSourceVoxelSize.factor * Vec3Double(mag)).toList)))
List(
NgffCoordinateTransformation(
scale = Some(List[Double](1.0) ++ (dataSourceVoxelSize.factor * Vec3Double(mag)).toList),
translation = None,
))
))
val lengthUnitStr = dataSourceVoxelSize.unit.toString
val axes = List(
Expand Down
6 changes: 4 additions & 2 deletions ...astore/app/com/scalableminds/webknossos/datastore/datareaders/zarr/NgffMetadataV0_5.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@ object NgffMetadataV0_5 {
mag =>
NgffDataset(
path = mag.toMagLiteral(allowScalar = true),
List(NgffCoordinateTransformation(
scale = Some(List[Double](1.0) ++ (dataSourceVoxelSize.factor * Vec3Double(mag)).toList)))
List(
NgffCoordinateTransformation(
scale = Some(List[Double](1.0) ++ (dataSourceVoxelSize.factor * Vec3Double(mag)).toList),
translation = None))
))
val lengthUnitStr = dataSourceVoxelSize.unit.toString
val axes = List(NgffAxis(name = "c", `type` = "channel")) ++ additionalAxes
Expand Down
4 changes: 3 additions & 1 deletion ...om/scalableminds/webknossos/datastore/datareaders/zarr/SharedNgffMetadataAttributes.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import com.scalableminds.webknossos.datastore.models.{LengthUnit, VoxelSize}
import net.liftweb.common.{Box, Failure, Full}
import play.api.libs.json.{Json, OFormat}

case class NgffCoordinateTransformation(`type`: String = "scale", scale: Option[List[Double]])
case class NgffCoordinateTransformation(`type`: String = "scale",
scale: Option[List[Double]],
translation: Option[List[Double]])

object NgffCoordinateTransformation {
implicit val jsonFormat: OFormat[NgffCoordinateTransformation] = Json.format[NgffCoordinateTransformation]
Expand Down
13 changes: 11 additions & 2 deletions ...ssos-datastore/app/com/scalableminds/webknossos/datastore/explore/ExploreLayerUtils.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,17 @@ trait ExploreLayerUtils extends FoxImplicits {
preferredVoxelSize: Option[VoxelSize],
voxelSize: VoxelSize): List[DataLayerWithMagLocators] =
layers.map(l => {
val coordinateTransformations = coordinateTransformationForVoxelSize(voxelSize, preferredVoxelSize)
l.mapped(coordinateTransformations = coordinateTransformations)
val generatedCoordinateTransformation = coordinateTransformationForVoxelSize(voxelSize, preferredVoxelSize)
val existingCoordinateTransformations = l.coordinateTransformations
val combinedCoordinateTransformations = existingCoordinateTransformations match {
// See https://github.com/scalableminds/webknossos/pull/8311/files/bbdde25dc4c1955281a45e5be063a8d5d1d8194c#r1913128610 for discussion.
// We are not merging the coordinate transformations, but rather appending the generated ones to the existing ones.
// It is unclear what the correct behavior should be, since we do not have test datasets where this would be relevant.
case Some(coordinateTransformations) =>
Some(coordinateTransformations ++ generatedCoordinateTransformation.getOrElse(List()))
case None => generatedCoordinateTransformation
}
l.mapped(coordinateTransformations = combinedCoordinateTransformations)
})

private def isPowerOfTwo(x: Int): Boolean =
Expand Down
42 changes: 42 additions & 0 deletions ...s-datastore/app/com/scalableminds/webknossos/datastore/explore/NgffExplorationUtils.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import com.scalableminds.webknossos.datastore.models.LengthUnit.LengthUnit
import com.scalableminds.webknossos.datastore.models.datasource.LayerViewConfiguration.LayerViewConfiguration
import com.scalableminds.webknossos.datastore.models.datasource.{
AdditionalAxis,
CoordinateTransformation,
CoordinateTransformationType,
DataLayerWithMagLocators,
ElementClass,
LayerViewConfiguration
Expand Down Expand Up @@ -259,6 +261,46 @@ trait NgffExplorationUtils extends FoxImplicits {
layerAndVoxelSizeTuples = layers.map((_, VoxelSize(voxelSizeFactor, unifiedAxisUnit)))
} yield layerAndVoxelSizeTuples

protected def getTranslation(multiscale: NgffMultiscalesItem): Option[List[CoordinateTransformation]] = {
val is2d = !multiscale.axes.exists(_.name == "z")
val baseTranslation = if (is2d) List(1.0, 1.0) else List(1.0, 1.0, 1.0)
multiscale.datasets.headOption match {
case Some(firstDataset) if firstDataset.coordinateTransformations.exists(_.`type` == "translation") => {
var translation = firstDataset.coordinateTransformations.foldLeft(baseTranslation)((acc, ct) => {
ct.`type` match {
case "translation" =>
ct.translation match {
case Some(translationList) =>
acc.zipWithIndex.map { case (v, i) => v * translationList(translationList.length - 1 - i) }
case _ => acc
Comment on lines +272 to +275
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

The review comment is valid and the suggested fix is necessary. The code assumes the translation list has sufficient elements without validation, which could lead to IndexOutOfBoundsException. The same unsafe pattern is also present in scale list handling. The suggested fix appropriately handles cases where the translation list is shorter than expected.

🔗 Analysis chain

Add validation for translation list length.

The code assumes the translation list has sufficient elements but doesn't validate this assumption. Consider adding length validation to prevent potential index out of bounds exceptions.

             case "translation" =>
               ct.translation match {
                 case Some(translationList) =>
+                  if (translationList.length < acc.length) acc
+                  else
                   acc.zipWithIndex.map { case (v, i) => v * translationList(translationList.length - 1 - i) }
                 case _ => acc
               }
🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found

Script:

#!/bin/bash
# Get the full context of the file
cat webknossos-datastore/app/com/scalableminds/webknossos/datastore/explore/NgffExplorationUtils.scala

# Look for test files related to NgffExplorationUtils
fd "NgffExplorationUtils.*Test.*scala" 

# Search for other usages of translation in the codebase
rg "translation" --type scala -A 2 -B 2

Length of output: 25250

Script:

#!/bin/bash
# Look for test files that might contain translation-related tests
rg -B 3 -A 3 "translation.*Test" --type scala

# Look for any GitHub issues mentioning translation and index
gh issue list -s all -S "translation index bounds"

# Search for potential error logs or exception handling related to translation
rg -B 2 -A 2 "IndexOutOfBoundsException.*translation" --type scala

Length of output: 169

Script:

#!/bin/bash
# Look for NgffCoordinateTransformation construction
rg "new NgffCoordinateTransformation|NgffCoordinateTransformation\(" --type scala -B 2 -A 2

# Look for similar array access patterns with length checks
rg "length.*\- 1.*\-.*i" --type scala -B 2 -A 2

# Look for validation patterns in NGFF code
rg "validate|check" --type scala --glob "*Ngff*.scala" -B 2 -A 2

Length of output: 4063

}
case _ =>
ct.scale match {
case Some(scaleList) => acc.zipWithIndex.map { case (v, i) => v / scaleList(scaleList.length - 1 - i) }
case _ => acc
}
}
})
if (is2d) {
translation = translation :+ 0.0
}
val xTranslation = translation(0)
val yTranslation = translation(1)
val zTranslation = translation(2)
val coordinateTransformation = CoordinateTransformation(
`type` = CoordinateTransformationType.affine,
matrix = Some(
List(List(1, 0, 0, xTranslation),
List(0, 1, 0, yTranslation),
List(0, 0, 1, zTranslation),
List(0, 0, 0, 1)))
)
Some(List(coordinateTransformation))
}
case _ => None
}
}

protected def layersForLabel(remotePath: VaultPath,
labelPath: String,
credentialId: Option[String]): Fox[List[(DataLayerWithMagLocators, VoxelSize)]]
Expand Down
3 changes: 3 additions & 0 deletions ...ossos-datastore/app/com/scalableminds/webknossos/datastore/explore/NgffV0_4Explorer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class NgffV0_4Explorer(implicit val ec: ExecutionContext) extends RemoteLayerExp
channelIndex)
boundingBox = boundingBoxFromMags(magsWithAttributes)
additionalAxes <- getAdditionalAxes(multiscale, remotePath)
translationOpt = getTranslation(multiscale)
layer: ZarrLayer = if (looksLikeSegmentationLayer(datasetName, elementClass) || isSegmentation) {
ZarrSegmentationLayer(
channelName,
Expand All @@ -67,6 +68,7 @@ class NgffV0_4Explorer(implicit val ec: ExecutionContext) extends RemoteLayerExp
largestSegmentId = None,
additionalAxes = Some(additionalAxes),
defaultViewConfiguration = Some(viewConfig),
coordinateTransformations = translationOpt,
dataFormat = DataFormat.zarr
)
} else
Expand All @@ -78,6 +80,7 @@ class NgffV0_4Explorer(implicit val ec: ExecutionContext) extends RemoteLayerExp
magsWithAttributes.map(_.mag),
additionalAxes = Some(additionalAxes),
defaultViewConfiguration = Some(viewConfig),
coordinateTransformations = translationOpt,
dataFormat = DataFormat.zarr
)
} yield layer
Expand Down
3 changes: 3 additions & 0 deletions ...ossos-datastore/app/com/scalableminds/webknossos/datastore/explore/NgffV0_5Explorer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class NgffV0_5Explorer(implicit val ec: ExecutionContext) extends RemoteLayerExp

boundingBox = boundingBoxFromMags(magsWithAttributes)
additionalAxes <- getAdditionalAxes(multiscale, remotePath)
translationOpt = getTranslation(multiscale)
layer: Zarr3Layer = if (looksLikeSegmentationLayer(datasetName, elementClass) || isSegmentation) {
Zarr3SegmentationLayer(
channelName,
Expand All @@ -70,6 +71,7 @@ class NgffV0_5Explorer(implicit val ec: ExecutionContext) extends RemoteLayerExp
largestSegmentId = None,
additionalAxes = Some(additionalAxes),
defaultViewConfiguration = Some(viewConfig),
coordinateTransformations = translationOpt,
)
} else
Zarr3DataLayer(
Expand All @@ -80,6 +82,7 @@ class NgffV0_5Explorer(implicit val ec: ExecutionContext) extends RemoteLayerExp
magsWithAttributes.map(_.mag),
additionalAxes = Some(additionalAxes),
defaultViewConfiguration = Some(viewConfig),
coordinateTransformations = translationOpt,
)
} yield layer

Expand Down