These seem to be the same as the classes in org.tensorflow.op.data?

I don't see a AssertCardinalityDataset class under org.tensorflow.op.data, is it what you meant?

Ooops, sorry I meant AssertNextDataset. There are a bunch of classes that are in both org.tensorflow.op.data and org.tensorflow.op.data.experimental.

These are two different kernels in TensorFlow runtime, like other duplicates found in these package as far as I can see. For example, the op under data is using AssertNextDataset while the other under experimental is using ExperimentalAssertNextDataset. So it is correct that they coexist, probably they achieve the same purpose but using different implementations. See here

Augment this method to handle a shape of UNKNOWN_SIZE? (The previous version accidentally handled it.)

In fact, passing a totally or partially unknown shape to this constructor should be forbidden, I'll add a check for this.

Document the behavior when shape has UNKNOWN_SIZE and -1 is passed for size?

same as in #174 (comment)

Given the new support for size of -1, perhaps change to "can be explicitly set".

Elsewhere (such as in Tensor), we simply call this type(). I do feel the emotional tug to be more explicit in this case, but I wonder if it will simply feel like inconsistency once we have lived with the new paradigm for a while.

I was hesitating on this one as well so sounds like we are two now, meaning that it should be type(), I'll rename it.

dType -> type throughout this file

!= would be more consistent.

rankDType -> rankType

This test code throws the expected exception because of the assignment to intTensor, rather than in asTypedTensor(). Here is alternative test code that also passes, but demonstrates that asTypedTensor() doesn't have its documented behavior:

  @Test
  public void rawToTypedTensor() {
    RawTensor rawTensor = RawTensor.allocate(TFloat32.class, Shape.of(2, 2), -1);
    TFloat32 floatTensor = rawTensor.asTypedTensor();
    assertSame(floatTensor.asRawTensor(), rawTensor);
    Object objTensor = rawTensor.<TInt32>asTypedTensor();
    try {
      TInt32 intTensor = (TInt32) objTensor;
      fail();
    } catch (ClassCastException e) {
      // ok
    }
  }

Good point, I've never tried that before. You want me to update the doc or try to throw an ClassCastException in this case as well?

Note that asTypedTensor() is an internal package-private method and where it is currently being used, it is implicitly enforced that the returned type matches the type of the tensor for all cases.

Updating the doc seems good to me for both of the reasons you give.

To be honest I'm not too sure how to document this. It looks to me that this behavior is some kind of "defect" in the generic specification, since the explicit <TInt32> parameterization of the method invocation seems to be overridden implicitly to the type inferred by the target (Object in this case).

So basically, <TInt32> is ignored and that is probably the case for Java type inference in general when dealing with type parameters inferred by a target, is it worth documenting it here then?

The type will be erased at runtime so the cast is erased to its bound (TType) which always succeeds. It might be better to have it explicitly return TType rather than have it promise something that can't be enforced by the type system. This is going to pollute the internals of our code with casts, but at least we'll remember to put them in rather than having it mysteriously blow up.

Ok sounds fair, let's do that, anyway all these additional casts will happen internally only.

Update: ended up that only one additional cast was required in the source code...

On further reflection, I lean the same way as @Craigacp: have this method return TType.

Here's some analysis to support this choice: At runtime, the current behavior is "apply the mapper from typeInfo and return whatever TType it produces." Since the type parameter T is erased at runtime, the only way to guarantee a return of T would be to have the method take the tensor type class as a runtime parameter and explicitly verify that the mapper returns that type class. I might advocate this approach for a public method or even a widely used package-private method, but not for a rarely used package-private method.

But also, a review of the call points of this method raises another issue: whenever we treat the return value of asTypedTensor as simply a Tensor (rather than a TType), we "forget" at compile time the semantic upgrade that was presumably the point of the asTypedTensor call in the first place.

As an example, let's explore what happens underneath the following method:

class EagerOperation extends AbstractOperation {
  // ...
  /**
   * Returns the tensor of the {@code outputIdx}th output of this operation.
   *
   * <p>This is only supported in an eager execution environment.
   *
   * @param outputIdx index of the output of this operation
   * @return output tensor
   */
  @Override
  Tensor tensor(int outputIndex) {
    Tensor tensor = outputTensors.get(outputIndex);
    if (tensor == null) {
      tensor = resolveTensor(outputIndex);
    }
    return tensor;
  }
  // ...
}

(I copied the Javadoc above the @Override for easy reference.)

The caller should presumably be agnostic as to whether this returns a RawTensor or a TType. But looking deeper into the call path, here's how the returned Tensor is constructed:

  private static Tensor resolveTensorHandle(TFE_TensorHandle handle, EagerSession session) {
    requireTensorHandle(handle);
    try (PointerScope scope = new PointerScope()) {
      TF_Status status = TF_Status.newStatus();
      TF_Tensor tensor = TFE_TensorHandleResolve(handle, status).withDeallocator();
      status.throwExceptionIfNotOK();
      return RawTensor.fromHandle(tensor, session).asTypedTensor();
    }
  }

What's the point of asTypedTensor() on the last substantive line, above? It causes this method to return a special kind of Tensor -- a TType -- but that fact is immediately forgotten by the type system and is not even asserted in the Javadoc. Presumably, we should either drop the call to .asTypedTensor() or change the whole call path to return TType.

:-) But not for this PR!

Add test to show intended behavior of shape with UNKNOWN_SIZE.

At some point we should fix the generator so it doesn't shadow the type name with a variable name, that's just confusing.

This is a place where it will infer the type bound from the calling context, and if that's wrong then we'll get a weird class cast error when people use it. It might be better to return the wildcard as at least the user will get a warning when they make the cast.

Again note that the TensorTypeRegistry is (for now) an internal class so we do have some control over the context it is being called. But like you've suggested, let's review this later.

Yeah, I know. But I'm in favour of having the compiler warn me before I make a silly mistake, and 6 months from now I won't necessarily remember that this method doesn't quite live up to its contract.