Better generic parameter type matching

[ctrueden]

When generics are involved, OPS does decent parameter type matching when deciding which op is the most appropriate for a given list of arguments. The reason it is decent, and not just crappy, is thanks to the great gentyref library, which helps tremendously for resolving when an object will actually safely fit into a given parameter field.

However, due to runtime generic type erasure, there are still cases where we cannot figure out whether the match is safe. For example:

public class AddConstantToArrayByteImage implements Op {
    @Parameter(type = ItemIO.BOTH)
    private ArrayImg<ByteType, ByteArray> image;

And:

public class AddConstantToArrayDoubleImage implements Op {
    @Parameter(type = ItemIO.BOTH)
    private ArrayImg<DoubleType, DoubleArray> image;

In this case, we cannot distinguish between an argument (i.e., object instance) of type ArrayImg<ByteType, ByteArray> and ArrayImg<DoubleType, DoubleArray> because at runtime, all that is known is that the argument is of type ArrayImg. But in practice it is backed by a particular type, e.g. ByteType, meaning that if it erroneously matches to the AddConstantToArrayDoubleImage, there will later be a ClassCastException when attempting to work with the object as though it were an ArrayImg<DoubleType, DoubleArray> (in this case, maybe when trying to assign the result of Cursor#get() to a DoubleType).

The currently proposed solution is to create a GenericTyped interface in SciJava Common with method Type[] getGenericTypes() that returns a list of Type objects matching the generic parameters. Or maybe it should return Class<?>[]... further thought and testing is needed. But the idea is to provide generically typed objects with a mechanism to "unerase" their types. Then the OPS matcher can check if the object implements this interface in order to glean its generic types and hence match to the op's inputs more precisely.

[ctrueden]

Another option for avoiding this issue completely would be to define subclasses for commonly used ImgLib2 types; e.g., ByteArrayImg extends ArrayImg<ByteType, ByteArray>. Then the matching would "just work" with no changes. I favor this plan for its simplicity, but am uncertain whether it might result in a proliferation of too many trivial "marker" subclasses in the ImgLib2 codebase.

[dscho]

I agree that the best place would be SciJava Common, but then we would have to force this dependency down the ImgLib2 team's throat because we need the ImgLib2 classes to implement it, right?

[ctrueden]

@dscho: Yes, ImgLib2 core would need to depend on SciJava Common then. We won't push for that until/unless this solution turns out to be the most technically advantageous, though.

[ctrueden]

When we get further along solving this issue, we can always ask @tpietzsch then which he would prefer: A) the marker subclasses, or B) implementing the GenericTyped interface (and adding a SciJava Common dependency). Or a third option, if we can come up with one.

[dscho]

Well, the problem is really that last time I tried to convince @tpietzsch to separate concerns, and put things unrelated to image processing into scijava-common, I managed to be unconvincing. It would be the clean solution, of course.

[tpietzsch]

My feeling is that marker subclasses would be a big big mess. Provisionally, I like GenericTyped better. Is anyone familiar with Guava or Guice? They seem to have similar things already: https://code.google.com/p/guava-libraries/wiki/ReflectionExplained http://google-guice.googlecode.com/git/javadoc/com/google/inject/TypeLiteral.html

[dscho]

@tpietzsch are you suggesting to add a Guava or Guice dependency now? Render me thoroughly puzzled.

[tpietzsch]

@dscho no. I'm just pointing out that these projects seem to have thought about the same problem and it might be useful to look at their solutions

[ctrueden]

@tpietzsch The Guava TypeToken looks like a good way to go for solving this. Thanks!

[ctrueden]

This is an old issue, but @Treiblesschorle is currently working on the next iteration of Ops that addresses generic type matching in a much better way. @Treiblesschorle, could you please add a status update here?

[Treiblesschorle]

In the new version of OPS, op requests need to provide the following information:

• OP name (String)
• Functional op type (Type). These are based on the java functional interfaces. E.g. we could ask for an 'add' function that adds doubles, which would be a BiFunction<Double, Double, Double>. This will be the type that is returned by the matcher.
• List of input types (Type[])
• List of output types (Type[])

Using the request, the op matcher loops over all available 'add' candidate ops and tries to match the types. This involves three things:

1. Check the generic assignability from the candidate functional type to the request functional type. Hence, we want to discern if it would be legal to create a new object of our candidate op and assign it to the requested type. E.g.:

BiFunction<Double, Double, Double> addOp = new OurOpCandidate();

Now, our candidate could be generic typed, e.g. we could have written an 'add' op that adds Numbers. Now the check also needs to figure out if java could be able to infer type arguments (new OurOpCandidate<>()). I tried to implement a method which does this making use of the Apache TypeUtils and GenTyRef. However, due to time constrains and me not being an expert in type inference algorithms, this may not give the correct answer in all the cases. However, it should be fine for the most common checks encountered in OPS.

2. Check type applicability of requested input types to the candidate input types. This check discerns whether it would be legal to call a method that requires a list of Types (which could contain generics) with a given list of Types. For this @ctrueden already started to implement such a check which I used and extended. However, it still might not cover all the cases correctly.

3. Check generic output assignability. This check discerns if the output types of our candidate can be assigned to the requested output types. This check is not yet fully implemented as I was not 100% sure how to do it. For now, only raw type assignability is checked and a warning is printed if several ops match with the same raw output types but different generic types. However, this should be fine for most cases.

Currently, we are in the process of converting the ops to the new system, which will show if the new type matching leads to problems.