Add support for RaggedTensors for TensorFlow backend #385
Conversation
| output = tf.random.stateless_categorical( | ||
| logits, num_samples, seed=seed | ||
| ) | ||
| output = tf.random.stateless_categorical(logits, num_samples, seed=seed) |
There was a problem hiding this comment.
Unrelated change made by the linter, please ignore.
keras_core/backend/torch/random.py
Outdated
| logits, | ||
| num_samples, | ||
| replacement=True, | ||
| generator=generator, |
There was a problem hiding this comment.
Unrelated change made by the linter, please ignore.
| @@ -826,3 +826,50 @@ def is_tpu_strat(k): | |||
| if is_tpu_strat(clz): | |||
| return True | |||
| return any(map(_is_tpu_strategy_class, clz.__bases__)) | |||
|
|
|||
|
|
|||
| # This function is taken from keras.engine.training.potentially_ragged_concat | |||
There was a problem hiding this comment.
The comment will be soon outdated (Keras Core will become Keras) and general Keras Core used a lot of code from tf.keras, so you can just remove the comment.
There was a problem hiding this comment.
Done, thanks!
keras_core/trainers/trainer_test.py
Outdated
| keras_core.backend.backend() != "tensorflow", | ||
| reason="Only tensorflow supports raggeds", | ||
| ) | ||
| def test_trainer_with_raggeds(self): |
There was a problem hiding this comment.
Please also test __call__ing a layer with ragged tensors and __calling__ing a Functional model and Sequential model since these are all different code paths
keras_core/trainers/trainer_test.py
Outdated
|
|
||
| model = ExampleModel() | ||
| x = tf.ragged.constant([[1], [2, 3]]) | ||
| model.compile(optimizer="adam") |
There was a problem hiding this comment.
How would it behave with losses and metrics? Can we reproduce the tf.keras behavior?
There was a problem hiding this comment.
How would it behave with losses and metrics?
Losses and Metrics don't work with RaggedTensors because of the way they are implemented in keras core. They can be supported but I'd leave that to a follow-up PR.
Will look into this. If possible, will create a follow-up PR. |
tirthasheshpatel
force-pushed
the
add-ragged-support
branch
from
59b4417
to
a951344
Compare
keras_core/trainers/trainer_test.py
Outdated
|
|
||
| class ExampleModel(base_class): | ||
| def __init__(self, input_shape=(None,)): | ||
| if base_class is keras_core.Functional: |
There was a problem hiding this comment.
This won't work for testing Functional because the call method is overridden. You'll need to use model = Functional(...) directly, without overriding call.
In this case you'll want to use a custom layer that supports Ragged (unless we already have layers that support Ragged...)
There was a problem hiding this comment.
Fixed. Right now, the tfnp API doesn't support RaggedTensors, so, the tests just make sure that keras_core allows raggeds as inputs and the outputs remain ragged. Let me know if you had something else in mind.
tirthasheshpatel
force-pushed
the
add-ragged-support
branch
from
a156f67
to
a62bdc9
Compare
| """Concats `Tensor`s along their first dimension. | ||
|
|
||
| Args: | ||
| tensors: List of `Tensor`s. |
| # model. We just test that passing RaggedTensors works for | ||
| # now. | ||
| outputs = inputs | ||
| super().__init__(inputs=inputs, outputs=outputs) |
There was a problem hiding this comment.
I think you want to use at least 1 layer to check it actually works. Just make a custom layer
There was a problem hiding this comment.
Done, thanks!
tirthasheshpatel
force-pushed
the
add-ragged-support
branch
from
744f360
to
f36c32b
Compare
|
Thanks for the reviews @fchollet! |
Keras Core didn't allow passing
RaggedTensors to methods ofkeras_core.Modeleven when using the Tensorflow backend. Since many models in Keras CV and Keras NLP have ragged tensors ingrained in thier APIs, it would be good to offer support for them at least when using Tensorflow as the backend.@ianstenbit