SavedModel Load Issue about neuropod HOT 2 CLOSED

The logics assume that the output tensor name is consistent with the feature name in output spec. The assumption is not correct.

The output tensor names must be consistent with the feature name in output spec. This is how Neuropod knows which tensor is which. This is a correct assumption because it's a requirement.

In Neuropod tensorflow backend, we actually map the tensor and the feature name based on the order.

When you say "based on the order", I assume you mean the order of things in the output spec. Please clarify if this isn't the ordering you're referring to.

The Neuropod TF backend does not depend on the order of items in the input/output spec.

As we've spoken about offline, we avoid depending on the order of items in the input/output spec because that introduces a brittle dependency on the order of items in the spec (same reason for not allowing return of a List[Tensor] or Tuple[Tensor] from TorchScript models). This can easily break, especially if there are several models using a centralized spec.

From what I can tell, the check you referenced in the issue is valid and necessary and we don't depend on the spec ordering in the TF backend. Feel free to comment if you were actually referring to something else.

I walked through some of the relevant parts of the code below and it should help clarify how inference with saved models works.

Here's a quick walkthrough of the relevant parts of the code:

1. The saved model is loaded and we set up a node name mapping that maps from the name of a Neuropod output to the corresponding node in the TF graph. For SavedModels, this is based on the signature of the saved model (for frozen graphs, it's explicitly specified).
   

   neuropod/source/neuropod/backends/tensorflow/tf_backend.hh

   Lines 48 to 49 in b18e281

   	// Map from a neuropod node name to the appropriate node in the TF graph
   	std::unordered_map<std::string, std::string> node_name_mapping_;

   
   

   neuropod/source/neuropod/backends/tensorflow/tf_backend.cc

   Lines 157 to 170 in b18e281

   	// Get the input and output node names for the `serving_default` signature in the savedmodel
   	// See https://www.tensorflow.org/guide/saved_model#specifying_signatures_during_export
   	// for more details
   	const auto &signature_def = bundle.GetSignatures().at("serving_default");
   	for (const auto &item : signature_def.inputs())
   	{
   	node_name_mapping_[item.first] = item.second.name();
   	}
   	for (const auto &item : signature_def.outputs())
   	{
   	node_name_mapping_[item.first] = item.second.name();
   	}

2. In infer, we set up our tensor_feeds and tensor_fetches. These are the inputs/outputs we want to use with a TF callable. More details about callables in the code snippet:

   neuropod/source/neuropod/backends/tensorflow/tf_backend.cc

   Lines 320 to 331 in b18e281

   	// In TensorFlow, a callable is a way of running a subgraph given a set of inputs and
   	// outputs. It's very similar to `session_->Run` except it has support for more fine-grained
   	// control over tensor devices. See https://github.com/tensorflow/tensorflow/issues/5902
   	// for more details.
   	// Fetches and feeds for our callable
   	// Note: these are ordered maps to make it easy to cache callables
   	// Map from an output node_name to an output_name
   	std::map<std::string, std::string> tensor_fetches;
   	// Map from an input node_name to a Tensor
   	std::map<std::string, tensorflow::Tensor> tensor_feeds;

3. We populate tensor_fetches (the outputs we want). Note that this is the step that would fail without the check you referenced in the issue
   

   neuropod/source/neuropod/backends/tensorflow/tf_backend.cc

   Lines 336 to 350 in b18e281

   	// Transform neuropod output names to node names in the graph
   	for (const auto &name : output_names)
   	{
   	const auto node_name = node_name_mapping_.find(name);
   	if (node_name == node_name_mapping_.end())
   	{
   	NEUROPOD_ERROR("Node {} not found in node_name_mapping. "
   	"Ensure that all items in the input/output spec have a corresponding item "
   	"in the node_name_mapping.",
   	name);
   	}
   	// Add this node name as an output of the subgraph we want to run
   	tensor_fetches.emplace(std::make_pair(node_name->second, name));
   	}

4. We get a callable with the feeds and fetches we populated

   neuropod/source/neuropod/backends/tensorflow/tf_backend.cc

   Line 373 in b18e281

   tensorflow::Session::CallableHandle handle = get_callable(tensor_feeds, tensor_fetches);

One thing to note here is that a callable is a way of running a TF subgraph given a set of inputs and outputs. What I think you may be referring to is that it takes an ordered list of feeds and an ordered list of fetches and accepts/produces Tensors in the same order. This order is based on tensor_feeds and tensor_fetches (which have consistent orderings because they're std::maps).

5. We loop over the outputs (which are in the same order as tensor_fetches) and return the output

   neuropod/source/neuropod/backends/tensorflow/tf_backend.cc

   Lines 387 to 396 in b18e281

   	// Read the outputs and wrap them in `NeuropodTensor`s
   	auto to_return = stdx::make_unique<NeuropodValueMap>();
   	size_t position = 0;
   	for (const auto &item : tensor_fetches)
   	{
   	const auto &output_name = item.second;
   	auto & output_tensor = outputs[position++];
   	const auto tensor_type = get_neuropod_type_from_tf_type(output_tensor.dtype());
   	(*to_return)[output_name] = make_tensor<TensorflowNeuropodTensor>(tensor_type, std::move(output_tensor));
   	}

So the "ordering" that it depends on is just an artifact of the way TF callables are run. It isn't based on anything outside of infer. We still need the names in the SavedModel output signature to match the names in the Neuropod output spec.

from neuropod.

Feel free to reopen I missed something, but I'll close this for now

from neuropod.