Skip to content

Commit 5941a41

Browse files
committed
Bugfix: Inconsistency in GWDTTRacer backed by SparseStorageBackend...
1 parent 1d62718 commit 5941a41

1 file changed

Lines changed: 33 additions & 8 deletions

File tree

‎src/main/java/sc/fiji/snt/tracing/auto/AbstractGWDTTracer.java‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ public void setMinSegmentLength(final double lengthInPhysicalUnits) {
280280
* it's considered redundant. Default: 0.4. Lower = more aggressive pruning.
281281
*/
282282
public void setSphereOverlapThreshold(final double threshold) {
283-
this.sphereOverlapThreshold = Math.max(0, Math.min(1, threshold));
283+
this.sphereOverlapThreshold = Math.clamp(threshold, 0, 1);
284284
}
285285

286286
/**
@@ -357,7 +357,7 @@ public void setResampleStep(final double step) {
357357
* @param type connectivity type (1, 2, or 3)
358358
*/
359359
public void setConnectivityType(final int type) {
360-
this.cnnType = Math.max(1, Math.min(3, type));
360+
this.cnnType = Math.clamp(type, 1, 3);
361361
}
362362

363363
/**
@@ -1729,7 +1729,7 @@ protected void recalculateRadiiFromImage(final DirectedWeightedGraph graph, fina
17291729

17301730
// Clamp to bounds
17311731
for (int d = 0; d < dims.length; d++) {
1732-
pos[d] = Math.max(0, Math.min(dims[d] - 1, pos[d]));
1732+
pos[d] = Math.clamp(pos[d], 0, dims[d] - 1);
17331733
}
17341734

17351735
sumOldRadius += node.radius;
@@ -2165,10 +2165,24 @@ protected void hierarchicalPrune(final DirectedWeightedGraph graph, final double
21652165
leaves.add(v);
21662166
}
21672167
}
2168-
leaves.sort((a, b) -> Double.compare(
2169-
distFromRoot.getOrDefault(b, 0.0),
2170-
distFromRoot.getOrDefault(a, 0.0)
2171-
));
2168+
// vertex insertion order in buildGraph(), which differs across StorageBackend implementations (e.g.,
2169+
// ArrayStorageBackend/ DiskBackedStorageBackend track ALIVE indices in a java.util.HashSet by default, while
2170+
// SparseStorageBackend falls back to iterating a fastutil Long2ByteOpenHashMap's keySet(), which is a
2171+
// different hash table with a different iteration order for the same keys). So we need to break by physical
2172+
// position makes the order an the pruning outcome independent of which backend (and which Set/Map implementation)
2173+
// produced the graph.
2174+
leaves.sort((a, b) -> {
2175+
final int cmp = Double.compare(
2176+
distFromRoot.getOrDefault(b, 0.0),
2177+
distFromRoot.getOrDefault(a, 0.0)
2178+
);
2179+
if (cmp != 0) return cmp;
2180+
int c = Double.compare(a.x, b.x);
2181+
if (c != 0) return c;
2182+
c = Double.compare(a.y, b.y);
2183+
if (c != 0) return c;
2184+
return Double.compare(a.z, b.z);
2185+
});
21722186

21732187
log("Processing " + leaves.size() + " leaves (longest paths first)");
21742188

@@ -2460,13 +2474,17 @@ protected void pruneParallelBranches(final DirectedWeightedGraph graph) {
24602474
while (changed) {
24612475
changed = false;
24622476

2463-
// Collect branch points (nodes with >1 outgoing edge)
2477+
// Same issue with fastutil in hierarchicalPrune: SparseStorageBackend falls back to iterating
2478+
// a fastutil hash map with a different iteration order
24642479
final List<SWCPoint> branchPoints = new ArrayList<>();
24652480
for (final SWCPoint v : graph.vertexSet()) {
24662481
if (graph.outDegreeOf(v) > 1) {
24672482
branchPoints.add(v);
24682483
}
24692484
}
2485+
branchPoints.sort(Comparator.comparingDouble((SWCPoint v) -> v.x)
2486+
.thenComparingDouble(v -> v.y)
2487+
.thenComparingDouble(v -> v.z));
24702488

24712489
outer:
24722490
for (final SWCPoint bp : branchPoints) {
@@ -2488,6 +2506,13 @@ protected void pruneParallelBranches(final DirectedWeightedGraph graph) {
24882506

24892507
if (stems.size() < 2) continue;
24902508

2509+
// Same determinism concern as branchPoints: graph.outgoingEdgesOf(bp)'s order
2510+
// is also backend-dependent, so canonicalize stem order before the pairwise
2511+
// comparison below picks which pair to evaluate (and potentially prune) first
2512+
stems.sort(Comparator.<List<SWCPoint>>comparingDouble(s -> s.getFirst().x)
2513+
.thenComparingDouble(s -> s.getFirst().y)
2514+
.thenComparingDouble(s -> s.getFirst().z));
2515+
24912516
// Compare every pair of sibling stems
24922517
for (int i = 0; i < stems.size(); i++) {
24932518
for (int j = i + 1; j < stems.size(); j++) {

0 commit comments

Comments
 (0)