Skip to content
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.

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

Fix work per thread in Outline #624

Closed
wants to merge 5 commits into from
Closed

Fix work per thread in Outline #624

wants to merge 5 commits into from

Conversation

rimadoma
Copy link
Contributor

@rimadoma rimadoma commented May 12, 2020

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
Copy link
Member

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

@rimadoma
Copy link
Contributor Author

@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
Copy link
Contributor

mdoube commented May 13, 2020

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 rimadoma marked this pull request as draft May 13, 2020 08:00
The op doesn't need its own ExecutorService. Plus the cachedThreadPool
in ThreadService ATM seems a bit faster for repeated runs.
@rimadoma rimadoma changed the title Revert Outline to single threaded execution Fix work per thread in Outline May 14, 2020
@rimadoma
Copy link
Contributor Author

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
Copy link
Contributor

mdoube commented Aug 25, 2021

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

@mdoube mdoube closed this Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fractal Dimension creates thousands of zombie threads that crash ImageJ
3 participants