Modify EI image repository and tag to match Python SDK. #12
Conversation
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
| @@ -39,6 +39,22 @@ function get_tfs_executable() { | |||
| rm ${zip_file} && rm -rf exec_dir | |||
| } | |||
|
|
|||
| function get_device_type() { | |||
There was a problem hiding this comment.
I think it would be less confusing to explicitly show that providing EI results in cpu being selected here.
There was a problem hiding this comment.
user has to provide eia/gpu/cpu arg to script so is this method even needed?
README.md
Outdated
| @@ -119,10 +119,10 @@ To test against Elastic Inference with Accelerator, you will need an AWS account | |||
| For example: | |||
|
|
|||
| pytest test/integrationsagemaker/test_elastic_inference.py --aws-id 0123456789012 \ | |||
There was a problem hiding this comment.
There is a / missing in between integration and sagemaker.
README.md
Outdated
| @@ -119,10 +119,10 @@ To test against Elastic Inference with Accelerator, you will need an AWS account | |||
| For example: | |||
|
|
|||
| pytest test/integrationsagemaker/test_elastic_inference.py --aws-id 0123456789012 \ | |||
| --docker-base-name sagemaker-tensorflow-serving \ | |||
| --docker-base-name sagemaker-tensorflow-serving-eia \ | |||
| --instance_type ml.m4.xlarge \ | |||
| --accelerator-type ml.eia1.large \ | |||
There was a problem hiding this comment.
let's default this to a medium for now.
scripts/shared.sh
Outdated
| } | ||
|
|
||
| function get_repository_name() { | ||
| if [ $1 = 'ei' ]; then |
There was a problem hiding this comment.
if we make user pass 'eia' instead of 'ei', then this is just an if/append
| @@ -39,6 +39,22 @@ function get_tfs_executable() { | |||
| rm ${zip_file} && rm -rf exec_dir | |||
| } | |||
|
|
|||
| function get_device_type() { | |||
There was a problem hiding this comment.
user has to provide eia/gpu/cpu arg to script so is this method even needed?
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Issue #, if available:
Description of changes:
Update image repository name and tag to be consistent with Python SDK.
https://github.com/aws/sagemaker-python-sdk/blob/master/src/sagemaker/fw_utils.py#L97
An EI image for TensorFlow looks like: 520713654638.dkr.ecr.us-west-2.amazonaws.com/sagemaker-tensorflow-eia:1.12-cpu-py2
The ECR repository it gets mapped to is sagemaker-tensorflow-eia, as it attaches eia at the end of non-eia repository, thus in this case should sagemaker-tensorflow-serving-eia.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.