tfdeploy icon indicating copy to clipboard operation
tfdeploy copied to clipboard

export_savedmodel for tfestimators gives warning

Open jjallaire opened this issue 8 years ago • 8 comments

If I add an export_savedmodel() call to this script: https://github.com/rstudio/tfestimators/blob/master/vignettes/examples/mnist.R

The I get the following warning after calling export_savedmodel:

WARNING:tensorflow:Export includes no default signature!

jjallaire avatar Jan 11 '18 17:01 jjallaire

This warning is ignorable, but not sure we would want to suppress this.

What I found out from tf.estimators.export_savedmodel is that the estimator definition is supposed to include export_outputs, for the linear_regressor() sources they don't. Which means that it does not explicitly include the "serving_default" signature which ends up triggering a warning here.

If my understanding is correct, we would rather need a change in tf.estimators over each canned estimator to include the proper serving function.

javierluraschi avatar Jan 12 '18 00:01 javierluraschi

So you are saying we need changes in TF itself? That won't be straightforward. I think we should either find a workaround for this or remove export_savedmodel() entirely for tfestimators (as I'm reading this we currently export the saved model but then can't serve it, is that correct?)

@kevinykuo Let us know your take on this.

jjallaire avatar Jan 12 '18 13:01 jjallaire

Aren't we actually calling the tf.estimator functions instead of the ones from tf.contrib.learn?

The model functions should be returning the appropriate export_outputs dict e.g. at https://github.com/tensorflow/tensorflow/blob/9bdb72e124e50e1b12b3286b38cbb1c971552741/tensorflow/python/estimator/canned/head.py#L660

Not sure why WARNING:tensorflow:Export includes no default signature! is coming up though

kevinykuo avatar Jan 12 '18 16:01 kevinykuo

@kevinykuo Not sure if I used the right term here, I meant canned as the non-custom ones, in this case, the example uses linear_regressor, would this estimator hit canned/head.py?

javierluraschi avatar Jan 12 '18 18:01 javierluraschi

@javierluraschi yeah linear_regressor is a canned estimator (in fact all estimators in tfestimators are canned except when you define your own via tfestimators::estimator()) and we hit canned/head.py here.

kevinykuo avatar Jan 12 '18 18:01 kevinykuo

@jjallaire / @kevinykuo alright, I finally understand what is going on, at least for LinearRegressor which is what I origanlly tried:

See:

https://github.com/tensorflow/tensorflow/blob/9bdb72e124e50e1b12b3286b38cbb1c971552741/tensorflow/python/estimator/canned/head.py#L654-L664

        classifier_output = _classification_output(
            scores=probabilities, n_classes=self._n_classes,
            label_vocabulary=self._label_vocabulary)
        return model_fn.EstimatorSpec(
            mode=model_fn.ModeKeys.PREDICT,
            predictions=predictions,
            export_outputs={
                _DEFAULT_SERVING_KEY: classifier_output,
                _CLASSIFY_SERVING_KEY: classifier_output,
                _PREDICT_SERVING_KEY: export_output.PredictOutput(predictions)
            })

That code is executing correctly and the estimator is returning 3 signatures, so in theory, when we call tfdeploy::serve_savedmodel() we should see 3 signatures to pick from.

However, the tf.estimators implementation of export_savedmodel() calls build_all_signature_defs which has this inner loop:

  for output_key, export_output in export_outputs.items():
    signature_name = '{}'.format(output_key or 'None')
    try:
      signature = export_output.as_signature_def(receiver_tensors)
      signature_def_map[signature_name] = signature
    except ValueError as e:
      excluded_signatures[signature_name] = str(e)

checking to see if the signature matches the tensor format, in this case, this is silently failing to all signatures but _PREDICT_SERVING_KEY since this is a prediction problem, not classification. This is borderline correct, the problem is just that the EstimatorSpec defaults to classification, not regression, for LinearRegressor, TF should implement a fix where they default to the prediction signature for prediction models, not classification.

Now taking a look at the MNIST.R issue, not LinearRegressor...

javierluraschi avatar Jan 22 '18 22:01 javierluraschi

@jjallaire Ah, right! Alright, so here is what is going on...

Back to the reference mentioned above, an estimator has 3 signatures:

return model_fn.EstimatorSpec(
            mode=model_fn.ModeKeys.PREDICT,
            predictions=predictions,
            export_outputs={
                _DEFAULT_SERVING_KEY: classifier_output,
                _CLASSIFY_SERVING_KEY: classifier_output,
                _PREDICT_SERVING_KEY: export_output.PredictOutput(predictions)
            })

The challenge with classifier_output is that the classification signature uses tf.Example which is the only option available in TF < 1.4. This is why we use build_parsing_serving_input_receiver_fn in that version; however, using build_parsing_serving_input_receiver_fn expects a tf.Examples which is some sort of protobof tensor encoded as a string really hard to craft without having the correct client.

Therefore, we default to always use raw tensors (not tf.Examples) but those happen to only match the prediction signature and the classification signatures are discarded.

Ideally, we would want to control create_estimator_spec and set serving_default as the prediction signature, not classification, but that requires a change in TF.

Might have some time to give this change a shot, but for now, we can safely ignore the warning.

javierluraschi avatar Jan 22 '18 23:01 javierluraschi

Could we surmount the warning if we required TF >= 1.4?

jjallaire avatar Jan 24 '18 11:01 jjallaire