[TypeRefact] Added Shaped interface shared across different containers

[karllessard]

PR related to #139.

It extracts the shape-related operators of the NdArray interface into a separate interface (Shaped) that is shared across the client for each container with a given shape.

I had to rename the output of a few operators as it was conflicting with the size() method of Shaped, which is now inherited by Operand.

CC\ @deansher

[deansher]

To me, this is a clear improvement. It does add a slight hand-wave to code like the following, but I lean toward believing that the previous, more explicit, version required just as much subtle knowledge as the new, more hand-wavy version.

Example of the old:

  public Operand<T> call(Operand<T> input) {
    Shape shape = input.asOutput().shape();

This now becomes:

  public Operand<T> call(Operand<T> input) {
    Shape shape = input.shape();

The old version explicitly showed it was talking about input's shape as an Output rather than as an eagerly evaluated Tensor. Arguably, that was good. But as someone who only recently learned this distinction (and who still feels slightly insecure on this point), I can say that .asOutput() didn't clarify this for me before I understood, and that it seems unnecessary now that I do.

I did suggest retaining overriding-class-specific javadoc in a couple of places that at least hints at this distinction.

[karllessard]

Thanks @deansher , I've updated the sources with your suggestions

[Craigacp]

LGTM.

However I did go and look at org.tensorflow.ndarray.Shape again. The computation of the size seems racy, and it stores it as a boxed Long rather than a primitive long. At the very least it should store it as a long. It might even be better to compute the shape size eagerly in the constructor, then the whole object becomes immutable.

[karllessard]

It might even be better to compute the shape size eagerly in the constructor, then the whole object becomes immutable.

That was my initial implementation, as it's the most intuitive one as well, but I've rolled back to that lazy-initialization of the size because surprisingly its computation had a bigger impact on my benchmark results that I was expecting.

We can revisit this later if you want but just to let you know that it is not a design mistake but a deliberated choice.

[Craigacp]

It might even be better to compute the shape size eagerly in the constructor, then the whole object becomes immutable.

That was my initial implementation, as it's the most intuitive one as well, but I've rolled back to that lazy-initialization of the size because surprisingly its computation had a bigger impact on my benchmark results that I was expecting.

We can revisit this later if you want but just to let you know that it is not a design mistake but a deliberated choice.

Ok, do you have the output of that benchmark before and after somewhere or should I run it myself?

[karllessard]

Ok, do you have the output of that benchmark before and after somewhere or should I run it myself?

Yes give it a try and let see what you observe on your side. If I remember correctly, pay more attention on this test