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

User provided node_name_mapping is not used with TF SavedModels #550

Open
VivekPanyam opened this issue Jun 3, 2022 · 0 comments
Open
Labels
good first issue Good for newcomers

Comments

@VivekPanyam
Copy link
Collaborator

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:

: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:

  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.

@VivekPanyam VivekPanyam added the good first issue Good for newcomers label Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant