feature: Add Framework Version support for PyTorch compilation (Neo) #2133

prateekch91 · 2021-02-10T21:33:36Z

Description of changes:
Add Framework Version support for PyTorch compilation (Neo), also added image URI support for 1.5 and 1.6. Updated 1.4.0 to 1.4 in image URI for consistency.

Example:
model.compile(
        target_instance_family="ml_c4",
        input_shape={"data": [1, 3, 1024, 1024]},
        output_path="s3://output",
        role="role",
        framework="pytorch",
        framework_version="1.6",
        job_name="compile-model",
)

Testing

Added test cases to test for different scenarios.
Tested Sagemaker compile for PyTorch versions - 1.4/1.5/1.6 with new Boto changes.

General

I have read the CONTRIBUTING doc
I used the commit message format described in CONTRIBUTING

Tests

[x ] 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 · 2021-02-10T21:42:47Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-10T22:16:03Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-10T22:16:44Z

AWS CodeBuild CI Report

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

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

lekk32 · 2021-02-10T22:57:00Z

src/sagemaker/model.py

@@ -407,6 +408,9 @@ def _compilation_job_config(
            else input_shape,
            "Framework": framework.upper(),
        }
+        if framework.upper() == "PYTORCH" and framework_version is not None:


Please add a check for target_device is a cloud ioc target. Framework version is not supported for edge devices and ml_inf1.

Addressed. Thanks!!

sagemaker-bot · 2021-02-10T23:05:20Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-11T00:59:12Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-11T01:02:41Z

AWS CodeBuild CI Report

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

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

kkoppolu1 · 2021-02-11T01:20:02Z

src/sagemaker/model.py

+                            framework_version
+                        )
+                    else:
+                        raise ValueError(


This seems to exclude deployment to ml_inf as well. Is that accurate?

Yes, when the model will not be compiled against this target, then there will be no deploy for this.

sagemaker-bot · 2021-02-11T01:32:01Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-11T01:32:35Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-11T01:34:58Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-11T01:35:19Z

AWS CodeBuild CI Report

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

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

lekk32

Looks good to me. Thanks.

sagemaker-bot · 2021-02-11T02:06:48Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-11T02:38:50Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-11T02:44:59Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-11T02:46:03Z

AWS CodeBuild CI Report

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

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

metrizable

Thanks for the contribution! A few questions:

Should you add an integration test?
Should you unit test more combinations of inputs for the _compilation_job_config method to ensure proper behavior (specifically when input_model_config["FrameworkVersion"] is set)?

metrizable · 2021-02-11T17:40:28Z

src/sagemaker/model.py

+        if framework.upper() == "PYTORCH" and framework_version is not None:
+            if target_instance_type is not None:
+                if target_instance_type.startswith("ml_"):
+                    if "ml_inf" not in target_instance_type:
+                        input_model_config["FrameworkVersion"] = utils.get_short_version(
+                            framework_version
+                        )


Flat is better than nested:

Suggested change

if framework.upper() == "PYTORCH" and framework_version is not None:

if target_instance_type is not None:

if target_instance_type.startswith("ml_"):

if "ml_inf" not in target_instance_type:

input_model_config["FrameworkVersion"] = utils.get_short_version(

framework_version

)

if (

framework.lower() == "pytorch" and

framework_version is not None and

target_instance_type is not None and

target_instance_type.startswith("ml_") and

"ml_inf" not in target_instance_type

):

input_model_config["FrameworkVersion"] = utils.get_short_version(framework_version)

If you imported re, you could also provide a regex for the last two. Something like:

re.match("(?=^ml_)(?!ml_inf)", target_instance_type) is not None

Thanks for the review!! I have updated the changes accordingly.

metrizable · 2021-02-11T17:42:11Z

src/sagemaker/model.py

+            framework_version (str):The version of framework, for example:
+                '1.5' for PyTorch


space after colon. also, use e.g.?

Suggested change

framework_version (str):The version of framework, for example:

'1.5' for PyTorch

framework_version (str): The version of framework (e.g. '1.5' for PyTorch)

Thanks!! Addressed.

sagemaker-bot · 2021-02-12T19:01:57Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-12T19:07:28Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-12T19:09:20Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-22T18:19:15Z

tests/data/pytorch_neo/code/inference.py

+        )
+
+    # preprocess image
+    decoded = Image.open(io.BytesIO(payload))


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

Analysis of this code determined that this line of code contains a resource that might not have closed properly. A resource leak can slow down or crash your system. Programs are strongly recommended to use the built in with keyword to open a resource or to use a try-finally block to open and close resources explicitly. The contextlib module provides helpful utilities for using the with statement.

Learn more

sagemaker-bot · 2021-02-22T18:24:38Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-22T18:52:30Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-22T18:52:52Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 2f35471
Result: FAILED
Build Logs (available for 30 days)

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

sagemaker-bot · 2021-02-22T18:55:26Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-22T19:02:13Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-23T18:58:17Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-23T19:25:30Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-23T19:32:59Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 12d669a
Result: FAILED
Build Logs (available for 30 days)

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

sagemaker-bot · 2021-02-23T19:34:09Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-23T19:34:10Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-24T18:54:08Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-24T19:27:12Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-24T19:28:33Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-24T19:30:29Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-24T20:06:29Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-24T20:36:03Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-24T20:43:04Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-24T20:43:50Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-24T21:10:50Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-25T16:49:26Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-25T17:12:38Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-25T17:17:34Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-25T17:24:07Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-02-25T17:41:11Z

AWS CodeBuild CI Report

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

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

lekk32 reviewed Feb 10, 2021

View reviewed changes

prateekch91 force-pushed the framework-versioning branch 2 times, most recently from 915f133 to 81f699c Compare February 11, 2021 00:53

kkoppolu1 reviewed Feb 11, 2021

View reviewed changes

prateekch91 force-pushed the framework-versioning branch from 81f699c to 00eda0b Compare February 11, 2021 01:57

kkoppolu1 previously approved these changes Feb 11, 2021

View reviewed changes

lekk32 previously approved these changes Feb 11, 2021

View reviewed changes

metrizable suggested changes Feb 11, 2021

View reviewed changes

prateekch91 dismissed stale reviews from lekk32 and kkoppolu1 via ce91758 February 12, 2021 18:58

prateekch91 force-pushed the framework-versioning branch 2 times, most recently from ce91758 to 4b0ff7a Compare February 12, 2021 19:00

prateekch91 force-pushed the framework-versioning branch from 4b0ff7a to cfff5b4 Compare February 12, 2021 19:06

sagemaker-bot reviewed Feb 22, 2021

View reviewed changes

Merge branch 'master' into framework-versioning

12d669a

Merge branch 'master' into framework-versioning

0c9baa3

Merge branch 'master' into framework-versioning

1b0ecee

Merge branch 'master' into framework-versioning

a245d45

ahsan-z-khan merged commit 2a0cf1b into aws:master Feb 25, 2021

		framework_version (str):The version of framework, for example:
		'1.5' for PyTorch

	framework_version (str):The version of framework, for example:
	'1.5' for PyTorch
	framework_version (str): The version of framework (e.g. '1.5' for PyTorch)

feature: Add Framework Version support for PyTorch compilation (Neo) #2133

feature: Add Framework Version support for PyTorch compilation (Neo) #2133

Uh oh!

Conversation

prateekch91 commented Feb 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing

General

Tests

Uh oh!

sagemaker-bot commented Feb 10, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 10, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 10, 2021

AWS CodeBuild CI Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Feb 10, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 11, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 11, 2021

AWS CodeBuild CI Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Feb 11, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 11, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 11, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 11, 2021

AWS CodeBuild CI Report

Uh oh!

lekk32 left a comment

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Feb 11, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 11, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 11, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 11, 2021

AWS CodeBuild CI Report

Uh oh!

metrizable left a comment

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!

sagemaker-bot commented Feb 12, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 12, 2021

AWS CodeBuild CI Report

Uh oh!

prateekch91 commented Feb 10, 2021 •

edited

Loading