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

OpListings ignore preallocated outputs #645

Open gselzer opened 2 years ago

gselzer commented 2 years ago

There are issues with the way I wrote OpListings. Shocker. 💥

imagej/napari-imagej#96 is a manifestation of one flaw of the current implementation: it does not retain any information about ItemIO.BOTH ModuleItems. This is made clear by the state held by the OpListing; we know the Op's inputs, and we know it's returns, but we don't know what gets mutated along the way. This limits the capabilities of the OpListing, and prevents us from accessing some functionality.

What I'd propose is instead to refactor the OpListing state to the following information:

EDIT: The above has been completed via #647

We may then also need to improve filtering step of OpSearcher.search() to allow for more corner cases. One case I had in mind might occur when two Ops share the same name, reduced pure inputs, and reduced outputs, but one is a Computer and the other is a Function. E.g. maybe for some number type T you have

BinaryComputer<ArrayImg<T> ,T, RAI<T>> math.add BinaryFunction<CellImg<T>, T, RAI<T>> math.add

In that case, I'd want to see something like math.add(img, number) -> img for both resulting reduced OpListings. Currently, we'd get two listings, as the current logic would suggest that the Computer has no return type, while the Function does, and thus the distinct() check in OpSearcher.search() would leave us with two OpListings :frowning:

But, we could get around this by being smarter, with the refactored state, and a better filter. All we really need to check to reduce N OpListings to one is to ensure that the first 4 of those new variables are the same; some idea of "union"s could allow for discrepancies in the last two variables to reduce to one OpListing.

I'll try writing a fix to the logic soon...