PreibischLab / RS-FISH

Tool for precise, interactive, fast and scalable FISH spot detection
GNU General Public License v2.0
45 stars 14 forks source link

Multithreading issues in 2D #27

Closed AlbertDominguez closed 3 months ago

AlbertDominguez commented 3 months ago

Hi! First, thanks for developing this great method!

When running benchmarks on 2D (YX) data with an ImageJ macro from the command line, I found the execution to completely stall without any warning. After some time (and seeing that the multithreaded version works very nicely on 3D data), I realised it has to do with the block sizes values.

Basically, the following two combinations do not work and cause the execution to stall and sometimes throw an OOM:

block_size_x = 512;
block_size_y = 512;
block_size_z = 1;
block_size_x = 1;
block_size_y = 512;
block_size_z = 512;

However, this one does work:

block_size_x = 512;
block_size_y = 1;
block_size_z = 512;

So basically, for 2D data, the program stalls without any warning if the sizes on Z or X are too small (<=2). Is this intended?

What confused me a bit is that I would have expected the dimensions to be XY/YX, not XZ/ZX (but to that end I have little to None experience with ImageJ, so might be my bad). In any case, it might be nice to add either a warning/Exception if there are inconsistencies with the array/input blocks.

StephanPreibisch commented 3 months ago

Great, thanks for reporting this @AlbertDominguez ... could you please share the cmd-line/ImageJ macro call that you used?

StephanPreibisch commented 3 months ago

In general, Java/ImageJ uses XYZ order ... but at least the same pixel origins as Python (there is a 0.5,0.5,1 xyz offset to Matlab compared to Java/Python)

StephanPreibisch commented 3 months ago

Hi, I found the bug ... a mix-up of array indices. And with a block size of 1 it will go into an infinite loop since it needs an overlap of 2 pixels. https://github.com/PreibischLab/RS-FISH/commit/5370293f55ad0cf738655d24e69535a197f3bf11

Great catch, thanks so much @AlbertDominguez

StephanPreibisch commented 3 months ago

And I also uploaded it to the Fiji update site if you'd like to check that it works now .... thanks @AlbertDominguez

AlbertDominguez commented 3 months ago

I've just tried it, and it works as expected!

Thanks a lot @StephanPreibisch :)