search

imagej / imagej-ops

ImageJ Ops: "Write once, run anywhere" image processing
https://imagej.net/libs/imagej-ops
BSD 2-Clause "Simplified" License
88 stars 42 forks source link

Fix work per thread in Outline #624

Closed rimadoma closed 3 years ago

rimadoma commented 4 years ago

In PR #618 @ctrueden reported Outline tests failing on a Linux box. Since neither of us could reproduce the issue in other environments and Travis CI was happy, I eventually shrugged it off as an isolated incident. Now @mdoube has reported failing tests in BoneJ, which depend on Outline. I think it's the same bug. This PR has been tested to fix the issue for him.

I don't have the resources to actually solve the bug right now (still can't reproduce). Rather than leave the issue lurking, I'm reverting the op to single threaded execution.

@ctrueden Could you please check whether this fixes the issue on the Linux box you mentioned?

frauzufall commented 4 years ago

@rimadoma just FYI I can't reproduce the issue on my Linux machine either.

rimadoma commented 4 years ago

@rimadoma just FYI I can't reproduce the issue on my Linux machine either.

Thanks! At least it's not failing on all Linux computers 😄

mdoube commented 4 years ago

I'm reverting the op to single threaded execution.

If I understand Outline correctly, it is just checking all pixels to see if they are a) foreground and b) have a background neighbour and if a) and b) are true keeping that pixel, otherwise setting it to background. Is that correct? If so it should be straightforward and robust to parallelise. It would however be a bit trickier to do 'in place' for both single and multi-threaded implementations rather than using a new output image for the result.

rimadoma commented 4 years ago

The bug was that for very small images (voxels / nThreads < 2) the work per thread was calculated incorrectly. The bug only started to break tests when nThreads >= 15.

mdoube commented 3 years ago

This version of Outline is still super-buggy throwing a large number of exceptions. Closing for now.

[ERROR] java.util.concurrent.ExecutionException: java.lang.ArrayIndexOutOfBoundsException
    at java.util.concurrent.FutureTask.report(FutureTask.java:122)
    at java.util.concurrent.FutureTask.get(FutureTask.java:192)
    at net.imagej.ops.morphology.outline.Outline.compute(Outline.java:121)
    at net.imagej.ops.morphology.outline.Outline.compute(Outline.java:61)
    at net.imagej.ops.special.hybrid.BinaryHybridCF.calculate(BinaryHybridCF.java:63)
    at net.imagej.ops.special.hybrid.BinaryHybridCF.calculate(BinaryHybridCF.java:85)
    at org.bonej.wrapperPlugins.FractalDimensionWrapper.lambda$run$0(FractalDimensionWrapper.java:178)
    at java.util.ArrayList.forEach(ArrayList.java:1257)
    at org.bonej.wrapperPlugins.FractalDimensionWrapper.run(FractalDimensionWrapper.java:174)
    at org.scijava.command.CommandModule.run(CommandModule.java:196)
    at org.scijava.module.ModuleRunner.run(ModuleRunner.java:163)
    at org.scijava.module.ModuleRunner.call(ModuleRunner.java:124)
    at org.scijava.module.ModuleRunner.call(ModuleRunner.java:63)
    at org.scijava.thread.DefaultThreadService.lambda$wrap$2(DefaultThreadService.java:225)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.ArrayIndexOutOfBoundsException