Skip to content

Commit 7f6e4ef

Browse files
committed
Add changes as discussed in PR #37:
- Add @deprecated to deprecated method - Revert to parallelizing over last dimension for deprecated method - Add method signatures with split dimension as parameter - Return `List` instead of `ArrayList` (deprecated method still returns `ArrayList`) The test cases have been made more comprehensive and miss only assertion statements and exception handling in the deprecated method.
1 parent 68803f9 commit 7f6e4ef

File tree

2 files changed

+185
-77
lines changed

2 files changed

+185
-77
lines changed

src/main/java/net/imglib2/algorithm/localextrema/LocalExtrema.java

Lines changed: 141 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,7 @@ public interface LocalNeighborhoodCheck< P, T >
103103
* test for being an extremum can be specified as an implementation of the
104104
* {@link LocalNeighborhoodCheck} interface.
105105
*
106-
* The task is parallelized along the longest dimension of
107-
* <code>source</code> after adjusting for size based on <code>shape</code>.
106+
* The task is parallelized along the last dimension of <code>source</code>.
108107
*
109108
* The number of tasks for parallelization is determined as:
110109
* <code>Math.max( Math.min( maxSizeDim, numThreads * 20 ), 1 )</code>
@@ -131,18 +130,27 @@ public interface LocalNeighborhoodCheck< P, T >
131130
* {@link ExecutorService} handles parallel tasks
132131
* @return {@link ArrayList} of extrema
133132
*/
133+
@Deprecated
134134
public static < P, T > ArrayList< P > findLocalExtrema( final RandomAccessibleInterval< T > source, final LocalNeighborhoodCheck< P, T > localNeighborhoodCheck, final ExecutorService service )
135135
{
136136
final RectangleShape shape = new RectangleShape( 1, true );
137137
final long[] borderSize = getRequiredBorderSize( shape, source.numDimensions() );
138138
final int nDim = source.numDimensions();
139+
final int splitDim = nDim - 1;
139140
// Get biggest dimension after border subtraction. Parallelize along
140141
// this dimension.
141-
final int maxSizeDim = IntStream.range( 0, nDim ).mapToObj( d -> new ValuePair<>( d, source.dimension( d ) - 2 * borderSize[ d ] ) ).max( ( p1, p2 ) -> Long.compare( p1.getB(), p2.getB() ) ).get().getA();
142142
final int numThreads = Runtime.getRuntime().availableProcessors();
143-
final int numTasks = Math.max( Math.min( maxSizeDim, numThreads * 20 ), 1 );
143+
final int numTasks = Math.max( Math.min( ( int ) shrink( source, borderSize ).dimension( splitDim ), numThreads * 20 ), 1 );
144144

145-
return findLocalExtrema( source, shrink( source, borderSize ), localNeighborhoodCheck, shape, service, numTasks );
145+
try
146+
{
147+
return ( ArrayList< P > ) findLocalExtrema( source, shrink( source, borderSize ), localNeighborhoodCheck, shape, service, numTasks, splitDim );
148+
}
149+
catch ( InterruptedException | ExecutionException e )
150+
{
151+
e.printStackTrace();
152+
return null;
153+
}
146154
}
147155

148156
/**
@@ -174,17 +182,63 @@ public static < P, T > ArrayList< P > findLocalExtrema( final RandomAccessibleIn
174182
* {@link ExecutorService} handles parallel tasks
175183
* @param numTasks
176184
* Number of tasks for parallel execution
177-
* @return {@link ArrayList} of extrema
185+
* @return {@link List} of extrema
186+
* @throws ExecutionException
187+
* @throws InterruptedException
178188
*/
179-
public static < P, T > ArrayList< P > findLocalExtrema(
189+
public static < P, T > List< P > findLocalExtrema(
180190
final RandomAccessibleInterval< T > source,
181191
final LocalNeighborhoodCheck< P, T > localNeighborhoodCheck,
182192
final Shape shape,
183193
final ExecutorService service,
184-
final int numTasks )
194+
final int numTasks ) throws InterruptedException, ExecutionException
195+
{
196+
final int splitDim = getBiggestDimension( shrink( source, getRequiredBorderSize( shape, source.numDimensions() ) ) );
197+
return findLocalExtrema( source, localNeighborhoodCheck, shape, service, numTasks, splitDim );
198+
}
199+
200+
/**
201+
* Find pixels that are extrema in their local neighborhood. The specific
202+
* test for being an extremum can be specified as an implementation of the
203+
* {@link LocalNeighborhoodCheck} interface.
204+
*
205+
* Note: Pixels within a margin of <code>source</code> border as determined
206+
* by {@link #getRequiredBorderSize(Shape, int)} will be ignored as local
207+
* extrema candidates because the complete neighborhood would not be
208+
* included in <code>source</code>. To include those pixel, expand
209+
* <code>source</code> accordingly. The returned coordinate list is valid
210+
* for the original <code>source</code>.
211+
*
212+
* @param source
213+
* Find local extrema within this
214+
* {@link RandomAccessibleInterval}
215+
* @param localNeighborhoodCheck
216+
* Check if current pixel qualifies as local maximum. It is the
217+
* callers responsibility to pass a
218+
* {@link LocalNeighborhoodCheck} that avoids the center pixel if
219+
* <code>shape</code> does not skip the center pixel.
220+
* @param shape
221+
* Defines the local neighborhood.
222+
* @param service
223+
* {@link ExecutorService} handles parallel tasks
224+
* @param numTasks
225+
* Number of tasks for parallel execution
226+
* @param splitDim
227+
* Dimension along which input should be split for parallization
228+
* @return {@link List} of extrema
229+
* @throws ExecutionException
230+
* @throws InterruptedException
231+
*/
232+
public static < P, T > List< P > findLocalExtrema(
233+
final RandomAccessibleInterval< T > source,
234+
final LocalNeighborhoodCheck< P, T > localNeighborhoodCheck,
235+
final Shape shape,
236+
final ExecutorService service,
237+
final int numTasks,
238+
final int splitDim ) throws InterruptedException, ExecutionException
185239
{
186240
final long[] borderSize = getRequiredBorderSize( shape, source.numDimensions() );
187-
return findLocalExtrema( source, shrink( source, borderSize ), localNeighborhoodCheck, shape, service, numTasks );
241+
return findLocalExtrema( source, shrink( source, borderSize ), localNeighborhoodCheck, shape, service, numTasks, splitDim );
188242
}
189243

190244
/**
@@ -213,69 +267,90 @@ public static < P, T > ArrayList< P > findLocalExtrema(
213267
* {@link ExecutorService} handles parallel tasks
214268
* @param numTasks
215269
* Number of tasks for parallel execution
216-
* @return {@link ArrayList} of extrema
270+
* @return {@link List} of extrema
271+
* @throws ExecutionException
272+
* @throws InterruptedException
217273
*/
218-
public static < P, T > ArrayList< P > findLocalExtrema(
274+
public static < P, T > List< P > findLocalExtrema(
219275
final RandomAccessible< T > source,
220276
final Interval interval,
221277
final LocalNeighborhoodCheck< P, T > localNeighborhoodCheck,
222278
final Shape shape,
223279
final ExecutorService service,
224-
final int numTasks )
280+
final int numTasks ) throws InterruptedException, ExecutionException
225281
{
282+
final int splitDim = getBiggestDimension( interval );
283+
return findLocalExtrema( source, interval, localNeighborhoodCheck, shape, service, numTasks, splitDim );
284+
}
226285

227-
final int nDim = source.numDimensions();
286+
/**
287+
* Find pixels that are extrema in their local neighborhood. The specific
288+
* test for being an extremum can be specified as an implementation of the
289+
* {@link LocalNeighborhoodCheck} interface.
290+
*
291+
* @param source
292+
* Find local extrema of the function defined by this
293+
* {@link RandomAccessible}
294+
* @param interval
295+
* Domain in which to look for local extrema. It is the callers
296+
* responsibility to ensure that <code>source</code> is defined
297+
* in all neighborhoods of <code>interval</code>.
298+
* @param localNeighborhoodCheck
299+
* Check if current pixel qualifies as local maximum. It is the
300+
* callers responsibility to pass a
301+
* {@link LocalNeighborhoodCheck} that avoids the center pixel if
302+
* <code>shape</code> does not skip the center pixel.
303+
* @param shape
304+
* Defines the local neighborhood.
305+
* @param service
306+
* {@link ExecutorService} handles parallel tasks
307+
* @param numTasks
308+
* Number of tasks for parallel execution
309+
* @param splitDim
310+
* Dimension along which input should be split for parallization
311+
* @return {@link List} of extrema
312+
* @throws ExecutionException
313+
* @throws InterruptedException
314+
*/
315+
public static < P, T > List< P > findLocalExtrema(
316+
final RandomAccessible< T > source,
317+
final Interval interval,
318+
final LocalNeighborhoodCheck< P, T > localNeighborhoodCheck,
319+
final Shape shape,
320+
final ExecutorService service,
321+
final int numTasks,
322+
final int splitDim ) throws InterruptedException, ExecutionException
323+
{
228324

229325
final long[] min = Intervals.minAsLongArray( interval );
230326
final long[] max = Intervals.maxAsLongArray( interval );
231327

232-
final int maxSizeDim = IntStream.range( 0, nDim ).mapToObj( d -> new ValuePair<>( d, interval.dimension( d ) ) ).max( ( p1, p2 ) -> Long.compare( p1.getB(), p2.getB() ) ).get().getA();
233-
final long maxDimSize = interval.dimension( maxSizeDim );
234-
final long maxDimMax = max[ maxSizeDim ];
235-
final long maxDimMin = min[ maxSizeDim ];
236-
final long taskSize = Math.max( maxDimSize / numTasks, 1 );
328+
final long splitDimSize = interval.dimension( splitDim );
329+
final long splitDimMax = max[ splitDim ];
330+
final long splitDimMin = min[ splitDim ];
331+
final long taskSize = Math.max( splitDimSize / numTasks, 1 );
237332

238-
final ArrayList< Callable< ArrayList< P > > > tasks = new ArrayList<>();
333+
final ArrayList< Callable< List< P > > > tasks = new ArrayList<>();
239334

240-
for ( long start = maxDimMin, stop = maxDimMin + taskSize - 1; start <= maxDimMax; start += taskSize, stop += taskSize )
335+
for ( long start = splitDimMin, stop = splitDimMin + taskSize - 1; start <= splitDimMax; start += taskSize, stop += taskSize )
241336
{
242337
final long s = start;
243338
// need max here instead of dimension for constructor of
244339
// FinalInterval
245-
final long S = Math.min( stop, maxDimMax );
340+
final long S = Math.min( stop, splitDimMax );
246341
tasks.add( () -> {
247342
final long[] localMin = min.clone();
248343
final long[] localMax = max.clone();
249-
localMin[ maxSizeDim ] = s;
250-
localMax[ maxSizeDim ] = S;
344+
localMin[ splitDim ] = s;
345+
localMax[ splitDim ] = S;
251346
return findLocalExtrema( source, new FinalInterval( localMin, localMax ), localNeighborhoodCheck, shape );
252347
} );
253348
}
254349

255350
final ArrayList< P > extrema = new ArrayList<>();
256-
// TODO It is probably better to throw exception than to use try/catch
257-
// block and return potentially incomplete/inconsistent list of extrema.
258-
// Returning an empty list on exception could be a compromise without
259-
// changing the interface.
260-
try
261-
{
262-
final List< Future< ArrayList< P > > > futures = service.invokeAll( tasks );
263-
for ( final Future< ArrayList< P > > f : futures )
264-
try
265-
{
266-
extrema.addAll( f.get() );
267-
}
268-
catch ( final ExecutionException e )
269-
{
270-
// TODO Auto-generated catch block
271-
e.printStackTrace();
272-
}
273-
}
274-
catch ( final InterruptedException e )
275-
{
276-
// TODO Auto-generated catch block
277-
e.printStackTrace();
278-
}
351+
final List< Future< List< P > > > futures = service.invokeAll( tasks );
352+
for ( final Future< List< P > > f : futures )
353+
extrema.addAll( f.get() );
279354
return extrema;
280355

281356
}
@@ -298,9 +373,9 @@ public static < P, T > ArrayList< P > findLocalExtrema(
298373
* {@link RandomAccessibleInterval}
299374
* @param localNeighborhoodCheck
300375
* Check if current pixel qualifies as local maximum.
301-
* @return {@link ArrayList} of extrema
376+
* @return {@link List} of extrema
302377
*/
303-
public static < P, T > ArrayList< P > findLocalExtrema(
378+
public static < P, T > List< P > findLocalExtrema(
304379
final RandomAccessibleInterval< T > source,
305380
final LocalNeighborhoodCheck< P, T > localNeighborhoodCheck )
306381
{
@@ -329,9 +404,9 @@ public static < P, T > ArrayList< P > findLocalExtrema(
329404
* <code>shape</code> does not skip the center pixel.
330405
* @param shape
331406
* Defines the local neighborhood
332-
* @return {@link ArrayList} of extrema
407+
* @return {@link List} of extrema
333408
*/
334-
public static < P, T > ArrayList< P > findLocalExtrema(
409+
public static < P, T > List< P > findLocalExtrema(
335410
final RandomAccessibleInterval< T > source,
336411
final LocalNeighborhoodCheck< P, T > localNeighborhoodCheck,
337412
final Shape shape )
@@ -362,9 +437,9 @@ public static < P, T > ArrayList< P > findLocalExtrema(
362437
* callers responsibility to pass a
363438
* {@link LocalNeighborhoodCheck} that avoids the center pixel if
364439
* <code>shape</code> does not skip the center pixel.
365-
* @return {@link ArrayList} of extrema
440+
* @return {@link List} of extrema
366441
*/
367-
public static < P, T > ArrayList< P > findLocalExtrema(
442+
public static < P, T > List< P > findLocalExtrema(
368443
final RandomAccessible< T > source,
369444
final Interval interval,
370445
final LocalNeighborhoodCheck< P, T > localNeighborhoodCheck )
@@ -388,9 +463,9 @@ public static < P, T > ArrayList< P > findLocalExtrema(
388463
* <code>shape</code> does not skip the center pixel.
389464
* @param shape
390465
* Defines the local neighborhood
391-
* @return {@link ArrayList} of extrema
466+
* @return {@link List} of extrema
392467
*/
393-
public static < P, T > ArrayList< P > findLocalExtrema(
468+
public static < P, T > List< P > findLocalExtrema(
394469
final RandomAccessible< T > source,
395470
final Interval interval,
396471
final LocalNeighborhoodCheck< P, T > localNeighborhoodCheck,
@@ -466,6 +541,18 @@ public static < T > IntervalView< T > shrink( final RandomAccessibleInterval< T
466541

467542
}
468543

544+
/**
545+
*
546+
* @param interval
547+
* @return The biggest dimension of interval.
548+
*/
549+
public static int getBiggestDimension( final Interval interval )
550+
{
551+
final int nDim = interval.numDimensions();
552+
final int splitDim = IntStream.range( 0, nDim ).mapToObj( d -> new ValuePair<>( d, interval.dimension( d ) ) ).max( ( p1, p2 ) -> Long.compare( p1.getB(), p2.getB() ) ).get().getA();
553+
return splitDim;
554+
}
555+
469556
/**
470557
* A {@link LocalNeighborhoodCheck} to test whether a pixel is a local
471558
* maximum. A pixel is considered a maximum if its value is greater than or

0 commit comments

Comments
 (0)