Skip to content
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

Remove tests for preprocessing inside a functional model #1175

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

mattdangerw
Copy link
Member

@mattdangerw mattdangerw commented Jul 25, 2023

This removes all testing for using string dtypes inside the call graph of a functional model. This will never work in a multi-backend fashion because torch and jax deliberately lack string dtypes.

There is really only one case where adding tokenization into the call graph of a model makes sense--inference in tensorflow. And for that use case, we should switch to recommending an export flow (like tf.keras.export.ExportArchive()). Ditching these tests will simplify the code base, speed up testing, and allow us to focus on workflows that apply to all backends.

Instead we, can focus on the follow options for applying tokenization:

  1. Automatically, via a Task and tf.data.
  2. Manually, with tf.data.
  3. Manually, in eager mode outside of a model.
  4. Inside an exported tf.function.

For the export flow, it would look something like:

export_archive = keras.export.ExportArchive()
export_archive.track(classifier)
export_archive.add_endpoint(
    name="serve",
    fn=lambda x: classifier(classifier.preprocessor(x)),
    input_signature=[tf.TensorSpec(shape=(None), dtype=tf.string)],
)
export_archive.write_out("serving_model")

Plenty of planning will still need to be done on export() flows for keras_core, but that's probably out of scope for this PR.

@mattdangerw
Copy link
Member Author

/gcbrun

Copy link
Contributor

@jbischof jbischof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mattdangerw mattdangerw merged commit 99e2a9a into keras-team:master Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants