import sagemaker.tensorflow is broken after upgrading TF serving

[zmjjmz]

Hey there,

So I recently upgraded my local tensorflow serving version to uh, 1.5:

(dataplayground2) zach@wa1okdba002:~/stats/omc/utils$ apt-cache show tensorflow-model-server           
Package: tensorflow-model-server
Version: 1.5.0
Architecture: all
Maintainer: TensorFlow Serving team
Priority: optional
Section: contrib/devel
Filename: pool/tensorflow-model-server/t/tensorflow-model-server/tensorflow-model-server_1.5.0_all.deb
Size: 89631204
SHA256: 4f25bde0f8dad88ec5184d060e5e10d2c15f1241977f72e50d233d5f7ddb6fa6
SHA1: a20e4e9d242cf874b12289ba9a8fb3cc97da7df7
MD5sum: 8867dad0c1258813a8a75b46ef8df536
Description: TensorFlow Serving ModelServer
Description-md5: 9b7e03f5296f318009581d6e285e2f89
Homepage: https://github.com/tensorflow/serving
Built-Using: Bazel

And my tensorflow-serving-api package as well:

tensorflow-serving-api==1.6.0

After this upgrade, I realized I can't run import sagemaker.tensorflow in Python using the latest sagemaker SDK version (1.2.2) (also the version that I had installed before I realized I hadn't updated it and did so in the vain hope of fixing this issue).

The exact issue I get is as follows:

(dataplayground2) zach@wa1okdba002:~/stats/omc/utils$ python -c "import sagemaker.tensorflow"
/home/u1/zach/proj/dataplayground2/local/lib/python2.7/site-packages/h5py/__init__.py:34: FutureWarning: Conversion of the second argum
ent of issubdtype from `float` to `np.floating` is deprecated. In future, it will be treated as `np.float64 == np.dtype(float).type`.
  from ._conv import register_converters as _register_converters
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/u1/zach/proj/dataplayground2/local/lib/python2.7/site-packages/sagemaker/tensorflow/__init__.py", line 28, in <module>
    from sagemaker.tensorflow.estimator import TensorFlow  # noqa: E402
  File "/home/u1/zach/proj/dataplayground2/local/lib/python2.7/site-packages/sagemaker/tensorflow/estimator.py", line 25, in <module>
    from sagemaker.tensorflow.model import TensorFlowModel
  File "/home/u1/zach/proj/dataplayground2/local/lib/python2.7/site-packages/sagemaker/tensorflow/model.py", line 18, in <module>
    from sagemaker.tensorflow.predictor import tf_json_serializer, tf_json_deserializer
  File "/home/u1/zach/proj/dataplayground2/local/lib/python2.7/site-packages/sagemaker/tensorflow/predictor.py", line 22, in <module>
    from tensorflow_serving.apis import predict_pb2, classification_pb2, inference_pb2, regression_pb2
  File "/home/u1/zach/proj/dataplayground2/local/lib/python2.7/site-packages/sagemaker/tensorflow/tensorflow_serving/apis/inference_pb2
.py", line 16, in <module>
    from tensorflow_serving.apis import classification_pb2 as tensorflow__serving_dot_apis_dot_classification__pb2
  File "/home/u1/zach/proj/dataplayground2/local/lib/python2.7/site-packages/tensorflow_serving/apis/classification_pb2.py", line 26, i
n <module>
    dependencies=[tensorflow__serving_dot_apis_dot_input__pb2.DESCRIPTOR,tensorflow__serving_dot_apis_dot_model__pb2.DESCRIPTOR,])
  File "/home/u1/zach/proj/dataplayground2/local/lib/python2.7/site-packages/google/protobuf/descriptor.py", line 829, in __new__
    return _message.default_pool.AddSerializedFile(serialized_pb)
TypeError: Couldn't build proto file into descriptor pool!
Invalid proto descriptor for file "tensorflow_serving/apis/classification.proto":
  tensorflow_serving/apis/classification.proto: A file with this name is already in the pool.

It seems to me like somehow it's trying to load that tensorflow_serving/apis/classification.proto twice, which is probably not intentional, although I'll note that I'm able to do this myself with no issues:

(dataplayground2) zach@wa1okdba002:~/stats/omc/utils$ python -c "from tensorflow_serving.apis import classification_pb2; from tensorflow_serving.apis import classification_pb2"
(dataplayground2) zach@wa1okdba002:~/stats/omc/utils$

At the moment this is entirely preventing me from using SageMaker through the Python SDK which is certainly a problem...

[zmjjmz]

Quick update: I tried this out a few times and can only replicate it with tensorflow-serving-api==1.6.0, lower versions do not cause this issue. I'm unsure at this point if this is an issue w/the sagemaker package or the tensorflow-serving-api package, but it seems to me like sagemaker might have been relying on deprecated / undocumented behavior that broke?

[lukmis]

Hi zmjjmz,

sorry for a long delay. Indeed I observe the same behavior. The reason behind this is that Python SDK has copies of the tensorflow-serving protobuf files (under src/sagemaker/tensorflow/tensorflow_serving). Version 1.6.0 has changed these files potentially making them back-incompatible from previous versions.

When you try to import from from tensorflow_serving.apis outside of the SDK it goes to the installed packages and works.

This will be analyzed and addressed by the team. Thank you for reporting this!

[mvsusp]

Hi @zmjjmz,

I investigated the issue today and got to the following conclusions:

1 - tensorflow-serving-api is only available in python 2 and cannot be installed using python 3. TensorFlow serving is not planning to create a python 3 version (tensorflow/serving#700) although the content of these versions would be the same. That is the main reason for us to maintain a copy of the protobuf messages inside SageMaker Python SDK.

2 - The issue occurs because Python 2 will prioritize loading the system installed module instead of the relative module version. That issue is solved adding future absolute import in any file that loads the reference. I enforced absolute imports in SageMaker to avoid it happening again: #180

3 - I have an additional PR updating the TF serving protobuf messages #181

I will merge these important changes as soon as possible to unblock you.

I appreciate you alerting us the issue and thanks for the patience.

Best,

Marcio

[mvsusp]

Hi @zmjjmz

The fix is merged in master. I will release the new version in pypi Monday.

Thanks for the patience.

[mvsusp]

Hi @zmjjmz

The release https://github.com/aws/sagemaker-python-sdk/releases/tag/v1.2.5 includes the fixes for the issue.

Thanks again for reporting this issue.