feature: Add Granular Target Description support for compilation (Neo) by apivovarov · Pull Request #1752 · aws/sagemaker-python-sdk

apivovarov · 2020-07-24T21:17:35Z

Description

Add Granular Target Description support for DL model compilation.
Example:

    model.compile(
        target_instance_family=None,
        input_shape={"data": [1, 3, 1024, 1024]},
        output_path="s3://output",
        role="role",
        framework="tensorflow",
        job_name="compile-model",
        target_platform_os="LINUX",
        target_platform_arch="ARM64",
        target_platform_accelerator="NVIDIA",
        compiler_options={"gpu-code": "sm_72", "trt-ver": "6.0.1", "cuda-ver": "10.1"}
    )

Tests

I have added tests that prove my fix is effective or that my feature works (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

sagemaker-bot · 2020-07-24T21:20:08Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: ffec986
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-24T21:44:57Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: ffec986
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-24T22:16:50Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: ffec986
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-24T22:24:54Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: ffec986
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-24T22:50:09Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: b42989f
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-24T23:16:30Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: b42989f
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-24T23:44:04Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: b42989f
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-25T00:40:30Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 6f5f6af
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-25T00:59:02Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 6f5f6af
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-25T01:04:53Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 6f5f6af
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-25T01:29:17Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 6f5f6af
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

src/sagemaker/estimator.py

laurenyu · 2020-07-27T16:05:45Z

src/sagemaker/model.py

+            assert target_platform_os is not None and target_platform_arch is not None, (
+                "target_instance_type or (target_platform_os and target_platform_arch) "
+                "should be provided"
+            )


it's better practice to avoid using asserts in source code, and instead raise a ValueError in this case

https://stackoverflow.com/a/945135
https://stackoverflow.com/a/1838411

Replaced with ValueError

laurenyu · 2020-07-27T16:07:26Z

src/sagemaker/model.py

                run your model after compilation, for example: ml_c5. For allowed
                strings see
                https://docs.aws.amazon.com/sagemaker/latest/dg/API_OutputConfig.html.
+                Alternatively, you can select OS, Arch and Accelerator using Target Platform


Suggested change

Alternatively, you can select OS, Arch and Accelerator using Target Platform

Alternatively, you can select an OS, Architecture, and Accelerator using

``target_platform_os``, ``target_platform_arch``, and ``target_platform_accelerator``.

fixed as suggested

laurenyu · 2020-07-27T16:08:29Z

src/sagemaker/model.py

+            target_platform_os (str): Target Platform OS. Allowed values: 'LINUX', 'ANDROID'.
+                It can be used instead of target_instance_family.
+            target_platform_arch (str): Target Platform Architecture. Allowed values: 'X86_64',
+                'X86', 'ARM64', 'ARM_EABIHF', 'ARM_EABI'.
+                It can be used instead of target_instance_family.
+            target_platform_accelerator (str): Target Platform Accelerator. Allowed values:
+                'INTEL_GRAPHICS', 'MALI', 'NVIDIA'. Optional.


can you instead link to docs that list the allowed versions? I assume they might change in the future, and if that happens, this docstring will inevitably be forgotten.

The API reference is updated:
OutputConfig API
TargetPlatform API

It would be good to provide these links as well.

I left one example value and added link to allowed values.

laurenyu · 2020-07-27T16:08:56Z

src/sagemaker/model.py

+                It can be used instead of target_instance_family.
+            target_platform_accelerator (str): Target Platform Accelerator. Allowed values:
+                'INTEL_GRAPHICS', 'MALI', 'NVIDIA'. Optional.
+            compiler_options (str): Additional parameters for compiler (Optional, in JSON format).


if it needs to be in JSON, would it make more sense to accept a dict, and then let the Python SDK call json.dumps?

Fixed, now it support both dict and string. Similar to DataInputConfig

laurenyu · 2020-07-27T16:11:12Z

src/sagemaker/model.py

+                "Devices described by Target Platform Os, Arch and Accelerator are not supported"
+                "to deploy via SageMaker, please deploy the model manually."


Suggested change

"Devices described by Target Platform Os, Arch and Accelerator are not supported"

"to deploy via SageMaker, please deploy the model manually."

"Deploying devices described by Target Platform OS, Architecture, and Accelerator "

"is not supported via SageMaker. Please deploy the model manually."

Can we keep old comment? I like it better.

"not supported to deploy" is the part that was bothering me. maybe "not supported for deployment"?

ok, changed to not supported for deployment.

laurenyu · 2020-07-27T16:12:09Z

src/sagemaker/model.py

+                    "The instance type %s is not supported to deploy via SageMaker,"
+                    "please deploy the model manually.",
+                    target_instance_family,


(I know this isn't your change, but while you're here...)

Suggested change

"The instance type %s is not supported to deploy via SageMaker,"

"please deploy the model manually.",

target_instance_family,

"Deploying instance type %s is not supported via SageMaker."

"Please deploy the model manually.",

target_instance_family,

Can we keep old comment? I like it better.

changed to not supported for deployment

tests/unit/sagemaker/model/test_neo.py

mchoi8739 · 2020-07-27T21:11:31Z

tests/unit/sagemaker/model/test_neo.py

+    model = _create_model(sagemaker_session)
+    model.compile(
+        target_instance_family=None,
+        input_shape={"data": [1, 3, 1024, 1024]},


input_shape={'data': [1, 3, 1024, 1024]}

^ suggesting to use single-quote (based on what we discussed earlier)

because of black-check python code style check we have to use double-quotes here

mchoi8739 · 2020-07-27T21:12:10Z

tests/unit/sagemaker/model/test_neo.py

+    model = _create_model(sagemaker_session)
+    model.compile(
+        target_instance_family=None,
+        input_shape={"data": [1, 3, 1024, 1024]},


input_shape={'data': [1, 3, 1024, 1024]}

^ suggesting to use single-quote (based on what we discussed earlier)

This is not JSON-like String which we use in Console UI or AWS CLI.
This is Python code. In Python SDK we use Dictionary {...} instead of JSON-string "{...}".
Technically we can use single quotes here. However, This project automatic build runs Python code style check (black-check) and it requires all strings to use double-quoutes for Strings in Python code.

mchoi8739 · 2020-07-27T21:25:23Z

src/sagemaker/model.py

+                It can be used instead of target_instance_family.
+            target_platform_accelerator (str): Target Platform Accelerator. Allowed values:
+                'INTEL_GRAPHICS', 'MALI', 'NVIDIA'. Optional.
+            compiler_options (str): Additional parameters for compiler (Optional, in JSON format).


We may need to point out here as well that target_instance_family and target_platform_* are not usable together?

For each target_platform_* field documentation I added - It can be used instead of target_instance_family.

sagemaker-bot · 2020-07-28T03:53:49Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 338dad1
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-28T04:10:28Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: c7fb5ed
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-28T04:19:55Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 338dad1
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-28T04:27:55Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: c7fb5ed
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-28T04:32:56Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: c7fb5ed
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-28T04:51:08Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 338dad1
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-28T04:58:25Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: c7fb5ed
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

laurenyu · 2020-07-28T15:58:15Z

src/sagemaker/model.py

+                compiler_options
+                if not isinstance(compiler_options, dict)
+                else json.dumps(compiler_options)


let's flip this around to put the positive condition first, i.e. json.dumps(compiler_options) if isinstance(compiler_options, dict) else compiler_options

fixed here and for DataInputConfig as well.

sagemaker-bot · 2020-07-28T19:57:13Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: fe8ba9d
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-28T20:17:15Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: fe8ba9d
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-28T20:44:42Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: fe8ba9d
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-28T21:45:42Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: fe8ba9d
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

apivovarov force-pushed the GTDD branch from ffec986 to b42989f Compare July 24, 2020 22:47

apivovarov force-pushed the GTDD branch from b42989f to 6f5f6af Compare July 25, 2020 00:29

laurenyu changed the title ~~neo: Add Granular Target Description support for compilation~~ feature: Add Granular Target Description support for compilation (Neo) Jul 27, 2020

laurenyu reviewed Jul 27, 2020

View reviewed changes

mchoi8739 reviewed Jul 27, 2020

View reviewed changes

apivovarov force-pushed the GTDD branch from 6f5f6af to 338dad1 Compare July 28, 2020 03:51

apivovarov force-pushed the GTDD branch from 338dad1 to c7fb5ed Compare July 28, 2020 04:01

laurenyu reviewed Jul 28, 2020

View reviewed changes

neo: Add Granular Target Description support for compilation

fe8ba9d

apivovarov force-pushed the GTDD branch from c7fb5ed to fe8ba9d Compare July 28, 2020 19:47

laurenyu approved these changes Jul 28, 2020

View reviewed changes

mchoi8739 approved these changes Jul 28, 2020

View reviewed changes

laurenyu merged commit 20751c9 into aws:master Jul 28, 2020

	Alternatively, you can select OS, Arch and Accelerator using Target Platform
	Alternatively, you can select an OS, Architecture, and Accelerator using
	``target_platform_os``, ``target_platform_arch``, and ``target_platform_accelerator``.

		"Devices described by Target Platform Os, Arch and Accelerator are not supported"
		"to deploy via SageMaker, please deploy the model manually."

Conversation

apivovarov commented Jul 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Tests

Uh oh!

sagemaker-bot commented Jul 24, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 24, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 24, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 24, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 24, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 24, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 24, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 25, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 25, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 25, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 25, 2020

AWS CodeBuild CI Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mchoi8739 Jul 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apivovarov Jul 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apivovarov Jul 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

apivovarov commented Jul 24, 2020 •

edited

Loading

mchoi8739 Jul 27, 2020 •

edited

Loading

apivovarov Jul 28, 2020 •

edited

Loading

apivovarov Jul 28, 2020 •

edited

Loading

apivovarov Jul 28, 2020 •

edited

Loading