-
Notifications
You must be signed in to change notification settings - Fork 214
Issues when translating from TType to TNumber and parameterized types within methods. #119
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
Comments
I'd be amazed and disappointed if the Java compiler conflated type variables of the same name across different scopes, such as if " I expect the main solution here is to be ruthlessly consistent in the specificity of our type constraints. I think we need to divide the parameters of our ops and framework methods into a few broad type categories, where both the compilation and the runtime support arbitrary mix and match within a category. To the extent at all possible, we need the compiler to complain if and only if an expression would fail at runtime. It's a major ergonomics loss when a compiler complaint can be resolved by a simple language-level cast like It would also be a major ergonomics loss if we had lots of categories of parameters with different runtime and compile-time behavior. Perhaps we could limit the number of distinct, commonly used categories to something like 3. E.g. |
Right now, the generated wrappers parametrize an operation to accept an Now note that I've already saw in the past cases where some types were wrong in the kernel definition (can't remember if it was that unsupported types were listed or if it was accepting all possible values while it should have been more restrictive). But basically, that is our source of truth. In your example @JimClarke5 , if the method
which is not bad in this case but if your condition applies to more than one type, it you need to go more generic:
I think this complexity could be partially resolved by the work in #92 again but it might not cover all cases yet, it is more focusing on tensors than any type of operands. |
I actually resolved this one by forcing everything to TNumber in the upcoming PR for losses. The issue mainly comes from TBool which is usually TF cast to a TNumber like TInt32 later in the method. However, sometimes the input might be a TNumber but still but still has to be passed as a TType in the method signature. This gets tricky when calling another method with |
I think this is the ideal scenario. It is more a headache for us, framework developers, to take care of this but that is how we can end up having compilation-time type checks that catches upfront all possible runtime errors.
Can you share an example? Normally if you call a method that accepts and returns |
The signature for If I do this with
This produces errors on :
If I try this, which is what I would like to do:
Errors with:
|
Here's my understanding of this situation. I haven't tried running my // T is an unknown subclass of TType. It could be a TString, a TBool, or a TNumber.
public static <T extends TType> Operand<T> handle(Ops tf, Operand<T> input) {
DataType<T> dataType = input.asOutput().dataType();
if(dataType.isBoolean()) {
// Since input is an Operand<T> and T may not be TInt32, this doesn't compile.
input = tf.dtypes.cast(input, TInt32.DTYPE);
}
// Since abs requires a TNumber and input could be something else, this doesn't compile.
return tf.math.abs(input);
}
// That didn't compile. Let's see what will:
@SuppressWarnings("unchecked")
public static <T extends TType> Operand<T> handle2(Ops tf, Operand<T> input) {
DataType<T> inputType = input.asOutput().dataType();
final Operand<? extends TNumber> x =
inputType.isBoolean()
? tf.dtypes.cast(input, TInt32.DTYPE)
: inputType.isString()
? tf.strings.toNumber((Operand<TString>) input)
: (Operand<? extends TNumber>) input;
// At this point, we know x is a TNumber, but we don't know its exact type at compile
// time. We also don't know the exact type of T at compile time, so it's not trivial
// to sort things out on the way back.
// Not all hope is lost, because the runtime value of input gave us the desired
// runtime DataType.
final Operand<? extends TNumber> a = tf.math.abs(x);
DataType<? extends TNumber> outputType = a.asOutput().dataType();
if (outputType == inputType) {
return (Operand<T>) a;
} else if (inputType.isString()) {
return (Operand<T>) tf.dtypes.asString(a);
} else {
return tf.dtypes.cast(a, inputType);
}
} |
Your example will result in untyped casts warnings. |
It does depend on the |
If we take the #139 route, many things around type handling will be different. But for this particular discussion here, it looks like everything works fine as expected so far so if you agree @JimClarke5 , I suggest that we close it and open a new one of problematic cases arise again? |
Many of the Java TF Ops use parameterized types for either
TType
,TNumber
or both. Sometimes anOp
uses<T extends TType>
and sometimes another Op is using<T extends TNumber>
. When writing a method that uses two different Ops that declare<T>
differently, the compiler complains thatT
cannot be converted to the other type. It is interesting thatTNumber
is a subclass ofTType
. I have searched "Professor Google", but have not found an answer to this kind of problem.TType
toTNumber
conversion is very common, especially if you are creating a base class with a common method signature across many similar objects. Sometimes, the subclass calls for aTType
, sometimes aTNumber
. The real problem, is when you have a common method such aspublic <T extends TType> Operand<T> call (Operand<T> input)
.As a work around, let's say that you cast a
TType
to aTNumber
(where<U extends TNumber>
) as in:Now when you call something like
tf.math.greater(uInput, otherValue);
, the compiler complains:no instance of type variables(s) exists so that T conforms to TNumber
. That is becausetf.math.greater
uses<T extends TNumber>
while other ops, liketf.nn.relu
defines<T extends TType>
.Another way around this is to force erasure as in
(Operand)value
.At a minimum, it would be nice if there were a convention like
<T extends TType>
and<U extends TNumber>
consistently, but this may not solve all these kind of issues, as I have seen<U extends TType, T extends TType>
, and<V extends TType, T extends TType, U extends TType>
The main issue that contributes to this problem is that the Ops require a mixture of types, so a higher level user is artificially juggling the situation by casting like above, or by forcing an erasure of the type.
IMO this situation is going to be confusing to the API user. I still haven't figured out a clean way to get around the issue when two method signatures use the same generic parameter in different ways.
Perhaps there is a better way. My gut feel is this is going to become a larger headache down the line.
The specific example I am running into at this time this problem is:
The text was updated successfully, but these errors were encountered: