search

imagej / imagej-omero

Server- and client-side communication between ImageJ and OMERO
GNU General Public License v2.0
11 stars 10 forks source link

Working with cell images from OMERO fails #92

Closed ctrueden closed 6 years ago

ctrueden commented 6 years ago

Install the following script into lib/Fiji.app/scripts/Plugins/Curtis/Frangi_Coolness.py:

#@ OpService ops
#@ Dataset image
#@output Img result
result = ops.run("create.img", image)
ops.filter().frangiVesselness(result, image, [1, 1] as double[], 20)

Then try running it on an image e.g. with omero script launch. The following error occurs:

Traceback (most recent call last):
  File "Plugins/Curtis/Frangi_Vessels.py", line 4, in <module>
Call setReader(Reader) before invoking create()
    at io.scif.img.cell.SCIFIOCellImgFactory.create(SCIFIOCellImgFactory.java:366)
    at io.scif.img.cell.SCIFIOCellImgFactory.create(SCIFIOCellImgFactory.java:92)
    at net.imagej.DefaultDataset$1.makeImgPlus(DefaultDataset.java:744)
    at net.imagej.DefaultDataset$1.create(DefaultDataset.java:731)
    at net.imagej.DefaultDataset$1.create(DefaultDataset.java:720)
    at net.imglib2.img.ImgFactory.create(ImgFactory.java:168)
    at net.imagej.ops.create.img.Imgs.create(Imgs.java:64)
    at net.imagej.ops.create.img.CreateImgFromImg.calculate(CreateImgFromImg.java:56)
    at net.imagej.ops.create.img.CreateImgFromImg.calculate(CreateImgFromImg.java:49)
    at net.imagej.ops.special.function.UnaryFunctionOp.run(UnaryFunctionOp.java:74)
    at net.imagej.ops.special.function.AbstractUnaryFunctionOp.run(AbstractUnaryFunctionOp.java:58)
    at org.scijava.command.CommandModule.run(CommandModule.java:199)
    at net.imagej.ops.OpEnvironment.run(OpEnvironment.java:944)
    at net.imagej.ops.OpEnvironment.run(OpEnvironment.java:135)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
java.lang.IllegalStateException: java.lang.IllegalStateException: Tried to create a new SCIFIOCellImg without a Reader to use for opening planes.
Call setReader(Reader) before invoking create()

    at org.python.core.Py.JavaError(Py.java:552)
    at org.python.core.Py.JavaError(Py.java:543)
    at org.python.core.PyReflectedFunction.__call__(PyReflectedFunction.java:190)
    at org.python.core.PyReflectedFunction.__call__(PyReflectedFunction.java:206)
    at org.python.core.PyObject.__call__(PyObject.java:515)
    at org.python.core.PyObject.__call__(PyObject.java:519)
    at org.python.core.PyMethod.__call__(PyMethod.java:156)
    at org.python.pycode._pyx0.f$0(Plugins/Curtis/Frangi_Vessels.py:5)
    at org.python.pycode._pyx0.call_function(Plugins/Curtis/Frangi_Vessels.py)
    at org.python.core.PyTableCode.call(PyTableCode.java:171)
    at org.python.core.PyCode.call(PyCode.java:18)
    at org.python.core.Py.runCode(Py.java:1614)
    at org.python.core.__builtin__.eval(__builtin__.java:497)
    at org.python.core.__builtin__.eval(__builtin__.java:501)
    at org.python.util.PythonInterpreter.eval(PythonInterpreter.java:259)
    at org.python.jsr223.PyScriptEngine.eval(PyScriptEngine.java:57)
    at org.python.jsr223.PyScriptEngine.eval(PyScriptEngine.java:31)
    at javax.script.AbstractScriptEngine.eval(AbstractScriptEngine.java:264)
    at org.scijava.script.ScriptModule.run(ScriptModule.java:160)
    at org.scijava.module.ModuleRunner.run(ModuleRunner.java:168)
    at org.scijava.module.ModuleRunner.call(ModuleRunner.java:127)
    at org.scijava.module.ModuleRunner.call(ModuleRunner.java:66)
    at org.scijava.thread.DefaultThreadService$3.call(DefaultThreadService.java:238)
    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.IllegalStateException: Tried to create a new SCIFIOCellImg without a Reader to use for opening planes.
Call setReader(Reader) before invoking create()
    at io.scif.img.cell.SCIFIOCellImgFactory.create(SCIFIOCellImgFactory.java:366)
    at io.scif.img.cell.SCIFIOCellImgFactory.create(SCIFIOCellImgFactory.java:92)
    at net.imagej.DefaultDataset$1.makeImgPlus(DefaultDataset.java:744)
    at net.imagej.DefaultDataset$1.create(DefaultDataset.java:731)
    at net.imagej.DefaultDataset$1.create(DefaultDataset.java:720)
    at net.imglib2.img.ImgFactory.create(ImgFactory.java:168)
    at net.imagej.ops.create.img.Imgs.create(Imgs.java:64)
    at net.imagej.ops.create.img.CreateImgFromImg.calculate(CreateImgFromImg.java:56)
    at net.imagej.ops.create.img.CreateImgFromImg.calculate(CreateImgFromImg.java:49)
    at net.imagej.ops.special.function.UnaryFunctionOp.run(UnaryFunctionOp.java:74)
    at net.imagej.ops.special.function.AbstractUnaryFunctionOp.run(AbstractUnaryFunctionOp.java:58)
    at org.scijava.command.CommandModule.run(CommandModule.java:199)
    at net.imagej.ops.OpEnvironment.run(OpEnvironment.java:944)
    at net.imagej.ops.OpEnvironment.run(OpEnvironment.java:135)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.python.core.PyReflectedFunction.__call__(PyReflectedFunction.java:188)
    ... 24 more

Seems that cell images are not playing nicely somehow.

ctrueden commented 6 years ago

The above exception is caused by the ops.run("create.img", image) call. Because SCIFIOCellImg.imgFactory().imgFactory(type) produces a new image factory that is uninitialized. It can be worked around (see next example).

Unfortunately, there are still problems. Consider the following script:

#@ OpService ops
#@ Dataset image
#@output Img result

outputType = image.firstElement()
imgFactory = net.imglib2.util.Util.getSuitableImgFactory(image, outputType)
result = ops.run("create.img", image, outputType, imgFactory)

ops.image().invert(result, image)

It produces the following error:

java.lang.RuntimeException: java.util.concurrent.ExecutionException: java.lang.RuntimeException: java.util.concurrent.ExecutionException: java.lang.ArrayIndexOutOfBoundsException
    at net.imagej.ops.thread.chunker.DefaultChunker.run(DefaultChunker.java:103)
    at org.scijava.command.CommandModule.run(CommandModule.java:199)
    at net.imagej.ops.OpEnvironment.run(OpEnvironment.java:944)
    at net.imagej.ops.OpEnvironment.run(OpEnvironment.java:156)
    at net.imagej.ops.map.MapUnaryComputers$IIToRAIParallel.compute(MapUnaryComputers.java:159)
    at net.imagej.ops.map.MapUnaryComputers$IIToRAIParallel.compute(MapUnaryComputers.java:145)
    at net.imagej.ops.image.invert.InvertII.compute(InvertII.java:87)
    at net.imagej.ops.image.invert.InvertII.compute(InvertII.java:47)
    at net.imagej.ops.special.computer.UnaryComputerOp.run(UnaryComputerOp.java:77)
    at net.imagej.ops.special.computer.UnaryComputerOp.run(UnaryComputerOp.java:92)
    at org.scijava.command.CommandModule.run(CommandModule.java:199)
    at net.imagej.ops.OpEnvironment.run(OpEnvironment.java:944)
    at net.imagej.ops.OpEnvironment.run(OpEnvironment.java:156)
    at net.imagej.ops.image.ImageNamespace.invert(ImageNamespace.java:263)
    at net.imagej.ops.image.ImageNamespace$invert.call(Unknown Source)
    at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:48)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:113)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:133)
    at Script1.run(Script1.groovy:9)
    at org.scijava.plugins.scripting.groovy.GroovyScriptEngine.eval(GroovyScriptEngine.java:303)
    at org.scijava.plugins.scripting.groovy.GroovyScriptEngine.eval(GroovyScriptEngine.java:122)
    at javax.script.AbstractScriptEngine.eval(AbstractScriptEngine.java:264)
    at org.scijava.script.ScriptModule.run(ScriptModule.java:160)
    at org.scijava.module.ModuleRunner.run(ModuleRunner.java:168)
    at org.scijava.module.ModuleRunner.call(ModuleRunner.java:127)
    at org.scijava.module.ModuleRunner.call(ModuleRunner.java:66)
    at org.scijava.thread.DefaultThreadService$3.call(DefaultThreadService.java:238)
    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.util.concurrent.ExecutionException: java.lang.RuntimeException: 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.thread.chunker.DefaultChunker.run(DefaultChunker.java:97)
    ... 30 more
Caused by: java.lang.RuntimeException: java.util.concurrent.ExecutionException: java.lang.ArrayIndexOutOfBoundsException
    at net.imglib2.cache.util.CacheAsUncheckedCacheAdapter.get(CacheAsUncheckedCacheAdapter.java:32)
    at net.imglib2.img.cell.LazyCellImg$LazyCells.get(LazyCellImg.java:104)
    at net.imglib2.img.list.AbstractLongListImg$LongListCursor.get(AbstractLongListImg.java:98)
    at net.imglib2.img.cell.CellLocalizingCursor.getCell(CellLocalizingCursor.java:105)
    at net.imglib2.img.cell.CellLocalizingCursor.jumpFwd(CellLocalizingCursor.java:141)
    at net.imagej.ops.map.Maps.map(Maps.java:310)
    at net.imagej.ops.map.MapUnaryComputers$IIToRAIParallel$1.execute(MapUnaryComputers.java:164)
    at net.imagej.ops.thread.chunker.DefaultChunker$2.run(DefaultChunker.java:87)
    at org.scijava.thread.DefaultThreadService$2.run(DefaultThreadService.java:221)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    ... 4 more
Caused by: java.util.concurrent.ExecutionException: java.lang.ArrayIndexOutOfBoundsException
    at net.imglib2.cache.ref.SoftRefLoaderRemoverCache.get(SoftRefLoaderRemoverCache.java:168)
    at net.imglib2.cache.util.LoaderRemoverCacheAsLoaderCacheAdapter.get(LoaderRemoverCacheAsLoaderCacheAdapter.java:37)
    at net.imglib2.cache.util.LoaderCacheAsCacheAdapter.get(LoaderCacheAsCacheAdapter.java:30)
    at net.imglib2.cache.util.CacheAsUncheckedCacheAdapter.get(CacheAsUncheckedCacheAdapter.java:28)
    ... 13 more
Caused by: java.lang.ArrayIndexOutOfBoundsException
    at java.lang.System.arraycopy(Native Method)
    at io.scif.util.ImageTools.splitChannels(ImageTools.java:237)
    at io.scif.filters.PlaneSeparator.openPlane(PlaneSeparator.java:321)
    at io.scif.filters.PlaneSeparator.openPlane(PlaneSeparator.java:207)
    at io.scif.filters.AbstractReaderFilter.openPlane(AbstractReaderFilter.java:224)
    at io.scif.filters.AbstractReaderFilter.openPlane(AbstractReaderFilter.java:190)
    at io.scif.img.cell.loaders.AbstractArrayLoader.read(AbstractArrayLoader.java:289)
    at io.scif.img.cell.loaders.AbstractArrayLoader.read(AbstractArrayLoader.java:271)
    at io.scif.img.cell.loaders.AbstractArrayLoader.read(AbstractArrayLoader.java:253)
    at io.scif.img.cell.loaders.AbstractArrayLoader.loadArray(AbstractArrayLoader.java:232)
    at io.scif.img.cell.SCIFIOCellImgFactory$SCIFIOCellLoader.load(SCIFIOCellImgFactory.java:204)
    at net.imglib2.cache.img.LoadedCellCacheLoader.get(LoadedCellCacheLoader.java:91)
    at net.imglib2.cache.img.LoadedCellCacheLoader.get(LoadedCellCacheLoader.java:51)
    at net.imglib2.cache.img.DiskCellCache.get(DiskCellCache.java:104)
    at net.imglib2.cache.img.DiskCellCache.get(DiskCellCache.java:43)
    at net.imglib2.cache.IoSync.get(IoSync.java:174)
    at net.imglib2.cache.ref.SoftRefLoaderRemoverCache.get(SoftRefLoaderRemoverCache.java:158)
    ... 16 more

Maybe an issue with concurrent access to cell images? @tpietzsch Should we expect concurrent access to such cell images to be possible? Or is it single-threaded only?

ctrueden commented 6 years ago

@gab1one Does this resemble the AIOOBE you had observed with cell images from before?

tpietzsch commented 6 years ago

Maybe an issue with concurrent access to cell images? @tpietzsch Should we expect concurrent access to such cell images to be possible? Or is it single-threaded only?

Yes, multi-threaded access should be definitely possible.

Is there a potential concurrency issue in the lower-level io.scif.img.cell.loaders.AbstractArrayLoader or io.scif.filters.AbstractReaderFilter. Because it is likely that these will be called from multiple threads concurrently. The cache just guarantees that the same cell will not be loaded by multiple threads. Different cells can be loaded concurrently.

tpietzsch commented 6 years ago

Hmm, actually, the io.scif.img.cell.loaders.AbstractArrayLoader.load(...) methods synchronize on the reader. So unless there are multiple readers that forward to the same shared resource in some way, there should be no problem.

tpietzsch commented 6 years ago

Desperate guess: To fit the existing AbstractArrayLoader to the cached CellImg mechanics, I made a variation of the method A loadArray(final int[] dimensions, final long[] min). This variation is void loadArray(final int[] dimensions, final long[] min, final A data), and this is used by the CellLoader. The difference is that the existing method creates the data array and returns it, while the new one gets the data array passed as an argument and fills it.

In a very weird world, data arrays that are created by the existing method could be bigger than the actual array required for a Cell, padded with nonsense in the back. (The CellImg would be fine with that, there is just no way to get at the additional data). In that case, the new method, being passed an array of exactly the required length, and passing it to the read method the same way as the old method, could actually trigger the above IndexOutOfBoundsException, because something might expect the additional padding.

It would be weird, but it's relatively easy to test: make the new method call the old one and fill the passed array with data from the array the old method returns. If that fixes the problem, ideally go down the rabbit hole and see if there is a fix for the places where the padding is required.

ctrueden commented 6 years ago

@tpietzsch Thank you for your thoughts.

We did find one bug introduced in scifio 0.36.1 (scifio/scifio@78ef0089ebba461654e84abdf09739117fc3745d), for which I have pushed a fix (scifio/scifio@064713feff237deec9a2dd1eb4ba2edcae7f9a6b). I am testing now to see if there continue to be other problems, or whether this was the root of the issue above.

ctrueden commented 6 years ago

It turns out that SCIFIO has inconsistent assumptions throughout the codebase regarding (long[], long[]) pairs meaning (min, max) or (offset, length). There are places where a (min, max) pair is handed to a method expecting (offset, length) and vice versa. There are places where the naming indicates (min, max) but an (offset, length) is computed/populated. It is pervasive. I have spent the last few hours converting all instances of these pairs throughout SCIFIO to use net.imglib2.Interval instead. This should alleviate the issue by imposing strong typing on the whole thing. But I am not yet finished. More tomorrow.

jburel commented 6 years ago

@ctrueden as discussed I hit the same problem when I open client-side the following image http://downloads.openmicroscopy.org/images/DV/siRNAi-HeLa/IN_02.r3d_D3D.dv

ctrueden commented 6 years ago

Thanks @jburel, I'll be sure to test your image before & after the code changes I am currently doing, to see how it fares.

ctrueden commented 6 years ago

I have finished migrating the SCIFIO API to use Interval instead of (long[], long[]) pairs. All compiles with passing tests; see scifio/scifio@min-max-bounds for details.

As of this writing, the status (on that topic branch) is as follows:

I will be mostly busy for the next few days, but will continue to prioritize getting this fully fixed and released.

ctrueden commented 6 years ago

I released imagej-omero version 0.6.0 and uploaded it to the OMERO-5.4 update site. @jburel Please give it a try and let me know how it works for you. On client side things appear to be working now in my tests. On server side scripts run, but we need to be careful about the return types. Returning Img does not seem sufficient to make the ModuleAdapter (OMERO server-side script runner) express it properly as an OMERO output image and commit it to the DB. You can probably tell whether a script is going to work w.r.t. its outputs by seeing what the type of the output is. If it says RString that is not correct—it needs to be RLong if I recall correctly.

jburel commented 6 years ago

It works with the DV. I will do further tests later on but so far it is looking good Thanks @ctrueden