Skip to content

Commit d7f1add

Browse files
tferrtferr
committed
Ensure AStarRefiner batches are not affected by settings changes mid run
1 parent c5ecaad commit d7f1add

4 files changed

Lines changed: 204 additions & 36 deletions

File tree

‎src/main/java/sc/fiji/snt/AStarRefiner.java‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,10 @@
3131
/**
3232
* Post-hoc A* re-tracing of a single, already-existing {@link Path}: treats the path's current
3333
* nodes as waypoints and re-derives the geometry between them via {@link SNT#autoTraceSync(List,
34-
* sc.fiji.snt.util.PointInImage)}, using whichever cost function/search parameters are currently
35-
* configured on the active {@link SNT} instance.
34+
* sc.fiji.snt.util.PointInImage, SNT.SearchSettingsSnapshot)}, using either the cost
35+
* function/search parameters currently configured on the active {@link SNT} instance, or a frozen
36+
* {@link SNT.SearchSettingsSnapshot} shared across a whole batch (see {@link #AStarRefiner(SNT,
37+
* Path, SNT.SearchSettingsSnapshot)}).
3638
* <p>
3739
* Intended for manually-traced paths against data streamed from disk/network, where running A*
3840
* interactively may be too slow to be practical. See {@code PathManagerUI}'s "Refine"
@@ -56,6 +58,7 @@ public class AStarRefiner implements Callable<Path> {
5658

5759
private final SNT snt;
5860
private final Path path;
61+
private final SNT.SearchSettingsSnapshot settings;
5962

6063
// Channel/frame the live SNT instance was set to when this refiner was built. Used as a
6164
// defensive backstop in call(): if these no longer match by the time the search completes,
@@ -69,18 +72,38 @@ public class AStarRefiner implements Callable<Path> {
6972
private String failureReason;
7073

7174
/**
72-
* Creates a refiner for a single path.
75+
* Creates a refiner for a single path, reading search settings (cost function, data structure,
76+
* secondary/hessian image) live from {@code snt} at the time {@link #call()} actually runs.
77+
* <p>
78+
* Prefer {@link #AStarRefiner(SNT, Path, SNT.SearchSettingsSnapshot)} for batches of more than
79+
* one path, so all paths share identical settings even if the live A* controls are changed
80+
* mid-batch (they remain enabled during a batch by design).
7381
*
7482
* @param snt the live SNT instance whose current A* search parameters (cost function,
7583
* hessian, image data) are used
7684
* @param path the path to re-trace; must have at least 2 nodes
7785
* @throws IllegalArgumentException if path is null or has fewer than 2 nodes
7886
*/
7987
public AStarRefiner(final SNT snt, final Path path) {
88+
this(snt, path, null);
89+
}
90+
91+
/**
92+
* Creates a refiner for a single path, using a frozen {@link SNT.SearchSettingsSnapshot} instead
93+
* of the live search settings.
94+
*
95+
* @param snt the live SNT instance whose image data is used
96+
* @param path the path to re-trace; must have at least 2 nodes
97+
* @param settings a snapshot from {@link SNT#snapshotSearchSettings()}, or null to read the live
98+
* settings at call-time (equivalent to {@link #AStarRefiner(SNT, Path)})
99+
* @throws IllegalArgumentException if path is null or has fewer than 2 nodes
100+
*/
101+
public AStarRefiner(final SNT snt, final Path path, final SNT.SearchSettingsSnapshot settings) {
80102
if (path == null || path.size() < 2)
81103
throw new IllegalArgumentException("Path must have at least 2 nodes");
82104
this.snt = snt;
83105
this.path = path;
106+
this.settings = settings;
84107
this.expectedChannel = snt.getChannel();
85108
this.expectedFrame = snt.getFrame();
86109
}
@@ -142,7 +165,7 @@ public Path call() {
142165
SNTUtils.log("AStarRefiner: Re-tracing '" + path.getName() + "' (" + path.size() + " waypoints)");
143166
try {
144167
final List<SNTPoint> waypoints = new ArrayList<>(path.getNodes());
145-
retraced = snt.autoTraceSync(waypoints, null);
168+
retraced = snt.autoTraceSync(waypoints, null, settings);
146169
if (retraced == null || retraced.size() < 2) {
147170
failureReason = "Search produced no result (out of bounds, or no path found through the data)";
148171
succeeded = false;

‎src/main/java/sc/fiji/snt/PathManagerUI.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1272,7 +1272,7 @@ private abstract class AbstractRefineHelper<T extends Callable<Path>> {
12721272

12731273
/**
12741274
* Whether the user should be prompted to adjust parameters. Returns
1275-
* {@code null} if the user cancelled, {@code true} to show the
1275+
* {@code null} if the user canceled, {@code true} to show the
12761276
* parameter dialog, {@code false} to proceed directly.
12771277
*/
12781278
abstract Boolean displayPromptRequired();
@@ -1331,7 +1331,7 @@ protected synchronized void cancel(final boolean updateUIState) {
13311331
void executeUsingPromptAsNeeded() {
13321332
if (workers == null || workers.isEmpty()) return;
13331333
final Boolean prompt = displayPromptRequired();
1334-
if (prompt == null) return; // user cancelled
1334+
if (prompt == null) return; // user canceled
13351335
if (prompt) {
13361336
showPromptAndThen(true);
13371337
} else {
@@ -5277,10 +5277,13 @@ public void execute(List<Path> selectedPaths, String cmd) {
52775277
}
52785278
selectedPaths = resolveActiveCTPathSelection(selectedPaths);
52795279
if (selectedPaths == null) return;
5280+
// Snapshot now, so every path in this batch shares identical settings, immune to
5281+
// changes made mid-run via the A* controls:
5282+
final SNT.SearchSettingsSnapshot settings = plugin.snapshotSearchSettings();
52805283
final ArrayList<AStarRefiner> refiners = new ArrayList<>();
52815284
for (final Path p : selectedPaths) {
52825285
if (p.size() >= 2)
5283-
refiners.add(new AStarRefiner(plugin, p));
5286+
refiners.add(new AStarRefiner(plugin, p, settings));
52845287
}
52855288
if (refiners.isEmpty()) {
52865289
guiUtils.error("No valid paths to re-trace (paths must have at least 2 nodes).");
@@ -5292,12 +5295,12 @@ public void execute(List<Path> selectedPaths, String cmd) {
52925295
+ "<li>Cost function: " + plugin.getCostType() + "</li>"
52935296
+ "<li>Secondary (filtered) image: "
52945297
+ (plugin.isTracingOnSecondaryImageActive() ? "In use (" + plugin.getFilterType() + ")" : "Not in use") + "</li>"
5295-
+ "</ul>Existing waypoints are kept; geometry between them is re-derived using a full A* search.",
5298+
+ "</ul>Existing waypoints are kept; geometry between them is re-derived using a full A* search. "
5299+
+ "These settings are locked in for the whole batch and won't be affected by later changes.",
52965300
"Confirm A* Re-trace")) {
52975301
return;
52985302
}
5299-
if (aStarRefineHelper == null)
5300-
aStarRefineHelper = new AStarRefineHelper();
5303+
aStarRefineHelper = new AStarRefineHelper();
53015304
aStarRefineHelper.refiners = refiners;
53025305
aStarRefineHelper.retraceUsingPromptAsNeeded();
53035306
}

‎src/main/java/sc/fiji/snt/SNT.java‎

Lines changed: 129 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2025,14 +2025,26 @@ private AbstractSearch createSearch(final double world_x_start,
20252025
final double world_x_end,
20262026
final double world_y_end,
20272027
final double world_z_end)
2028+
{
2029+
return createSearch(world_x_start, world_y_start, world_z_start, world_x_end, world_y_end, world_z_end, null);
2030+
}
2031+
2032+
private AbstractSearch createSearch(final double world_x_start,
2033+
final double world_y_start,
2034+
final double world_z_start,
2035+
final double world_x_end,
2036+
final double world_y_end,
2037+
final double world_z_end,
2038+
final SearchSettingsSnapshot settings)
20282039
{
20292040
return createSearch(
20302041
(int) Math.round(world_x_start / x_spacing),
20312042
(int) Math.round(world_y_start / y_spacing),
20322043
(int) Math.round(world_z_start / z_spacing),
20332044
(int) Math.round(world_x_end / x_spacing),
20342045
(int) Math.round(world_y_end / y_spacing),
2035-
(int) Math.round(world_z_end / z_spacing));
2046+
(int) Math.round(world_z_end / z_spacing),
2047+
settings);
20362048
}
20372049

20382050
/* This method uses the plugin's current search parameters to construct an isolated A* search instance using
@@ -2045,8 +2057,60 @@ private <T extends RealType<T>> AbstractSearch createSearch(final int x_start,
20452057
final int y_end,
20462058
final int z_end)
20472059
{
2060+
return createSearch(x_start, y_start, z_start, x_end, y_end, z_end, null);
2061+
}
20482062

2049-
final boolean useSecondary = isTracingOnSecondaryImageActive();
2063+
/**
2064+
* Immutable snapshot of the search settings that {@link #createSearch} would otherwise read
2065+
* live from the enclosing {@link SNT} instance ({@link #costType}, {@link #searchImageType},
2066+
* whether tracing on the secondary/filtered image is active).
2067+
* <p>
2068+
* Interactive, click-driven tracing is meant to always use whatever is currently configured, so
2069+
* it never needs this: {@link #createSearch} falls back to the live fields when no snapshot is
2070+
* given. Batch callers that run over a long, possibly unattended period (e.g. {@code
2071+
* AStarRefiner}, invoked from {@code PathManagerUI}'s "Re-trace with A*...") should instead take
2072+
* one snapshot when the batch is queued and reuse it for every path/segment, so a setting
2073+
* changed mid-run can't silently produce a batch with inconsistent per-path settings.
2074+
*
2075+
* @see #snapshotSearchSettings()
2076+
* @see #autoTraceSync(List, PointInImage, SearchSettingsSnapshot)
2077+
*/
2078+
public static final class SearchSettingsSnapshot {
2079+
final CostType costType;
2080+
final SearchImageType searchImageType;
2081+
final boolean useSecondary;
2082+
2083+
private SearchSettingsSnapshot(final CostType costType, final SearchImageType searchImageType,
2084+
final boolean useSecondary) {
2085+
this.costType = costType;
2086+
this.searchImageType = searchImageType;
2087+
this.useSecondary = useSecondary;
2088+
}
2089+
}
2090+
2091+
/**
2092+
* Captures the search settings currently configured on this instance, for use by long-running
2093+
* batch callers that need to stay internally consistent even if these settings are changed
2094+
* (via the UI) partway through the run. See {@link SearchSettingsSnapshot}.
2095+
*
2096+
* @return an immutable snapshot of the current cost function, data structure, and secondary
2097+
* (filtered) image state
2098+
*/
2099+
public SearchSettingsSnapshot snapshotSearchSettings() {
2100+
return new SearchSettingsSnapshot(costType, searchImageType, isTracingOnSecondaryImageActive());
2101+
}
2102+
2103+
private <T extends RealType<T>> AbstractSearch createSearch(final int x_start,
2104+
final int y_start,
2105+
final int z_start,
2106+
final int x_end,
2107+
final int y_end,
2108+
final int z_end,
2109+
final SearchSettingsSnapshot settings)
2110+
{
2111+
final boolean useSecondary = (settings != null) ? settings.useSecondary : isTracingOnSecondaryImageActive();
2112+
final CostType effCostType = (settings != null) ? settings.costType : costType;
2113+
final SearchImageType effSearchImageType = (settings != null) ? settings.searchImageType : searchImageType;
20502114

20512115
final RandomAccessibleInterval<T> img = useSecondary ? getSecondaryData() : getLoadedData();
20522116

@@ -2059,11 +2123,11 @@ private <T extends RealType<T>> AbstractSearch createSearch(final int x_start,
20592123
// and those fields are shared SNT-instance state: mutating them here would race across threads!
20602124
imgStats = computeImgStats(
20612125
ImgUtils.subInterval(img, new Point(x_start, y_start, z_start), new Point(x_end, y_end, z_end), 10),
2062-
new ImageStatistics(), costType);
2126+
new ImageStatistics(), effCostType);
20632127
}
20642128

20652129
Cost costFunction;
2066-
switch (costType)
2130+
switch (effCostType)
20672131
{
20682132
case RECIPROCAL:
20692133
costFunction = new Reciprocal(imgStats.min, imgStats.max);
@@ -2080,7 +2144,7 @@ private <T extends RealType<T>> AbstractSearch createSearch(final int x_start,
20802144
costFunction = new DifferenceSq(imgStats.min, imgStats.max);
20812145
break;
20822146
default:
2083-
throw new IllegalArgumentException("BUG: Unknown cost function " + costType);
2147+
throw new IllegalArgumentException("BUG: Unknown cost function " + effCostType);
20842148
}
20852149

20862150
Heuristic heuristic = switch (heuristicType) {
@@ -2089,27 +2153,66 @@ private <T extends RealType<T>> AbstractSearch createSearch(final int x_start,
20892153
default -> throw new IllegalArgumentException("BUG: Unknown heuristic " + heuristicType);
20902154
};
20912155

2156+
if (settings == null) {
2157+
// Interactive tracing
2158+
return switch (searchType) {
2159+
case ASTAR -> new TracerThread(
2160+
this,
2161+
img,
2162+
x_start,
2163+
y_start,
2164+
z_start,
2165+
x_end,
2166+
y_end,
2167+
z_end,
2168+
costFunction,
2169+
heuristic);
2170+
case NBASTAR -> new BiSearch(
2171+
this,
2172+
img,
2173+
x_start,
2174+
y_start,
2175+
z_start,
2176+
x_end,
2177+
y_end,
2178+
z_end,
2179+
costFunction,
2180+
heuristic);
2181+
default -> throw new IllegalArgumentException("BUG: Unknown search class");
2182+
};
2183+
}
2184+
2185+
// Batch caller with a frozen snapshot: use the explicit-parameter constructors so
2186+
// effSearchImageType is honored instead of whatever searchImageType currently is live.
2187+
// timeoutSeconds=0, reportEveryMilliseconds=1000 match the defaults the SNT-aware
2188+
// constructors above use internally (see AbstractSearch(SNT, RandomAccessibleInterval)).
20922189
return switch (searchType) {
20932190
case ASTAR -> new TracerThread(
2094-
this,
20952191
img,
2192+
getCalibration(),
20962193
x_start,
20972194
y_start,
20982195
z_start,
20992196
x_end,
21002197
y_end,
21012198
z_end,
2199+
0,
2200+
1000,
2201+
effSearchImageType,
21022202
costFunction,
21032203
heuristic);
21042204
case NBASTAR -> new BiSearch(
2105-
this,
21062205
img,
2206+
getCalibration(),
21072207
x_start,
21082208
y_start,
21092209
z_start,
21102210
x_end,
21112211
y_end,
21122212
z_end,
2213+
0,
2214+
1000,
2215+
effSearchImageType,
21132216
costFunction,
21142217
heuristic);
21152218
default -> throw new IllegalArgumentException("BUG: Unknown search class");
@@ -2531,14 +2634,32 @@ private Path autoTraceHeadless(final List<SNTPoint> pointList, final PointInImag
25312634
* @return the stitched path, or null if any segment failed to produce a result
25322635
*/
25332636
public Path autoTraceSync(final List<SNTPoint> pointList, final PointInImage forkPoint) {
2637+
return autoTraceSync(pointList, forkPoint, null);
2638+
}
2639+
2640+
/**
2641+
* Variant of {@link #autoTraceSync(List, PointInImage)} that uses a frozen
2642+
* {@link SearchSettingsSnapshot} instead of the live cost function/data structure/secondary-image
2643+
* settings, so every segment of every path in a long-running batch (e.g. {@code AStarRefiner})
2644+
* is traced with identical settings, immune to changes made mid-run via the (deliberately still
2645+
* enabled) A* controls.
2646+
*
2647+
* @param pointList the waypoints to connect, start to end
2648+
* @param forkPoint optional fork point of the parent Path, or null
2649+
* @param settings a snapshot from {@link #snapshotSearchSettings()}, or null to read the live
2650+
* settings (equivalent to {@link #autoTraceSync(List, PointInImage)})
2651+
* @return the stitched path, or null if any segment failed to produce a result
2652+
*/
2653+
public Path autoTraceSync(final List<SNTPoint> pointList, final PointInImage forkPoint,
2654+
final SearchSettingsSnapshot settings) {
25342655
if (pointList == null || pointList.isEmpty())
25352656
throw new IllegalArgumentException("pointList cannot be null or empty");
25362657
final Path fullPath = new Path(x_spacing, y_spacing, z_spacing, spacing_units);
25372658
for (int i = 0; i < pointList.size() - 1; i++) {
25382659
final SNTPoint start = pointList.get(i);
25392660
final SNTPoint end = pointList.get(i + 1);
25402661
final AbstractSearch search = createSearch(start.getX(), start.getY(), start.getZ(),
2541-
end.getX(), end.getY(), end.getZ());
2662+
end.getX(), end.getY(), end.getZ(), settings);
25422663
search.run(); // synchronous: runs on the calling (already-background) thread
25432664
final Path pathResult = search.getResult();
25442665
if (pathResult == null) {

0 commit comments

Comments
 (0)