search

uber / neuropod

A uniform interface to run deep learning models from multiple frameworks
https://neuropod.ai
Apache License 2.0
936 stars 77 forks source link

User provided `node_name_mapping` is not used with TF SavedModels #550

Open VivekPanyam opened 2 years ago

VivekPanyam commented 2 years ago

Context

The user provided node_name_mapping is not used when running TF SavedModels. It's only used when running TF frozen graphs.

TF SavedModels have SignaureDefs. These are quite similar to what node_name_mapping is in Neuropod.

Since SavedModel already has this functionality (and other projects like TF serving use it), it made sense to use that data as the node name mapping. Otherwise we’d either have to support mappings in both TF SignatureDefs and Neuropod or just Neuropod. The latter is not ideal and the former wasn't necessary for our initial SavedModel usecase.

Unfortunately, this not documented anywhere in create_tensorflow_neuropod and the docs are even a little misleading: https://github.com/uber/neuropod/blob/b18e281f3d29c91d2a43e4291abbe099ff2e852b/source/python/neuropod/backends/tensorflow/packager.py#L41-L52

Expected behavior

We should do the following:

  1. Make the docstring linked above explicitly say that node_name_mapping is not supported when using a SavedModel and that users should use TF SignaureDefs instead for that functionality.
  2. Throw an error if node_name_mapping is provided when packaging a SavedModel

Alternatively, we could add support for node_name_mapping with SavedModels, but we should decide whether that's behavior we want.