-
Notifications
You must be signed in to change notification settings - Fork 75
Description
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:
neuropod/source/python/neuropod/backends/tensorflow/packager.py
Lines 41 to 52 in b18e281
| :param node_name_mapping: Mapping from a neuropod input/output name to a node in the graph. The `:0` is | |
| optional. Required unless using a saved model. | |
| !!! note "" | |
| ***Example***: | |
| ``` | |
| { | |
| "x": "some_namespace/in_x:0", | |
| "y": "some_namespace/in_y:0", | |
| "out": "some_namespace/out:0", | |
| } | |
| ``` |
Expected behavior
We should do the following:
- Make the docstring linked above explicitly say that
node_name_mappingis not supported when using a SavedModel and that users should use TF SignaureDefs instead for that functionality. - Throw an error if
node_name_mappingis 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.