[Question]: The Tensors class should not implement an implicit cast to Tensor

[DevNullx64]

Description

The Tensors class implements an implicit cast to Tensor. It simply returns the first Tensor in its list. There is a potential risk of information loss in this case. While casting from float to double does not pose any problems, casting from double to float requires an explicit conversion.

Alternatives

I looked into the impact of an explicit cast. I made the necessary corrections to pass the tests. However, I had to modify the unit tests to ensure proper casting of Tensors. It doesn't feel clean to do that...

I went a bit further and realized that almost all implementations of the layers use a Tensor as input and output. I got overwhelmed by the task of revising everything quickly. Then, I reached the RNN section, which seems to be under development. At that point, I couldn't make the necessary corrections or add a comment to guide the changes. I gave up on completing the task when I realized the extent of the impact. I was in the process of making a modification that would affect everything derived from Loss or Layer, and with generics (to avoid having to declare typed arguments and hiding inherited members, etc.).

[AsakusaRinne]

@Oceania2018 any ideas about this?

[DevNullx64]

There is nothing insurmountable, I believe. Classes working with a tensor and returning a tensor should have a common abstract class inheriting from Layer. It would virtually and naively implement the Call method for each tensor input and associate it with its output. There are few layers that really need to implement tensors as inputs. They could keep their own implementations.

My main concern is that the modification will still change a lot of things in the keras.layer directory of the TensorFlow.Keras project.

I'm not an expert in TensorFlow or Keras, so forgive me if I'm mistaken.

[AsakusaRinne]

Thank you a lot for telling us this concern. I agree that the implicit cast from Tensors to Tensor is risky. To me this proposal hits a key problem and sounds very reasonable. I approve removing this implicit conversion in the future. Here're some of my thoughts of the design of Tensors and welcome to discuss it together.

1. The Apply of the layer, which is corresponding to __call__ in python, should keep using Tensors because it will make APIs consistent. Otherwise users may need to figure out whether the Apply with Tensors or the one with Tensor should be used for some layers.
2. Originally I wanted to delay this change to v1.0.0 to avoid break change in minor version. However when developing RNN recently, I found it better to change some behaviors of Tensors. Therefore I wonder if making this modification in v0.100.6 will impact much on users.
3. Also when developing RNN, I found sometimes the output of a layer is nested tensors, for example, [t1, [t2, t3]], and it's expected to be used as var (value, states) = tensors. This pushed me to re-consider the design of Tensors. Supporting nested structure in Tensors may introduce complexities to users, while letting users deconstruct a Tensors themselves may also be annoying. If the first one is taken as the solution finally, it's a good time to modify it together with the implicit conversion.

[DevNullx64]

I fully agree your first point. From user side, everything should work with tensors as input or ouput. Tensorflow.Net need to take it into account internally, from Tensorflow.Net developpers pov. For the second point, most impact should be on contributor. If some one activelly work on an impacted surface by the change, delaying seem to be a good. For the 3 point I also see some code with strange using of Tensors. But as I said, without Keras and tensor flow expertise I can see if it's functional or "experimental". You're welcom to discuss. I think I need to meet some one on discord or other.

Verry, I just want to have Model that support Tensors as y. And I need to understand a lot of thing within the code to do this change if no one do it ^^'. And I will do...

[AsakusaRinne]

Thank you for your suggestions. I'm working on the refactoring of Tensors now together with RNN. I'll let you review the code after I complete it.

Verry, I just want to have Model that support Tensors as y.

Sorry that I didn't get that point. Could you please give me an example?

[DevNullx64]

I have a model that have one Tensor input, but two tensor output. Something like:

var inputs = keras.Input(7, dtype: TF_DataType.TF_FLOAT); var outputs_pred = tf.keras.layers.Dense(3, activation: "relu").Apply(inputs); var outputs_pred2 = tf.keras.layers.Dense(3, activation: "relu").Apply(inputs); var model = keras.Model(inputs, (outputs_pred, outputs_pred2));

model.summary() and model.predict() seem to work as expected. But I've find no way to use model.evaluate() nor model.fit().

Because I don't know how to do, I read the code and try to have this to be fixed :p. And when reading I see a lot a litle thing that I want refactor: Class architecture, adding some generic, using of static reaonly string instead of constant or literal, add a string enumeration class to have "typed string" (on the back end at last), remove empty array creation, and so on and so forth ^^'...

[AsakusaRinne]

Yes, currently the solution for multi-outputs is outputting an Tensors or Tensor[]. I agree sometimes that may not be enough.

tensorflow.net is far from perfect and is progressing. Since v1.0.0 could include break changes, it'll be okay to refactor some parts. We'll be sincerely glad If you'd like to improve it with us together. If you would like so, what about inviting you to our discord dev channel? (though it's newly opened) :)