fix: use workflow parameters in training hyperparameters (#2114) (#2115) #2227

nmadan · 2021-03-19T20:25:38Z

Issue #, if available:
** #2114
** #2115

Description of changes:
**Changed Training job hyperparameter parsing logic to use SageMaker Pipeline parameters as is, instead of converting them to strings
** Changed Processing job input source uri parsing logic to ignore SageMaker Pipeline parameters

Testing done:
Edited unit tests to cover pipeline parameters for training and processing jobs

Merge Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.

General

[ x] I have read the CONTRIBUTING doc
[ x] I used the commit message format described in CONTRIBUTING
I have passed the region in to all S3 and STS clients that I've initialized as part of this change.
I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

[ x] I have added tests that prove my fix is effective or that my feature works (if appropriate)
I have checked that my tests are not configured for a specific region or account (if appropriate)
I have used unique_name_from_base to create resource names in integ tests (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-03-19T20:35:27Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-19T20:58:30Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-19T20:59:29Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-19T21:10:00Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-19T21:26:26Z

AWS CodeBuild CI Report

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

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

staubhp · 2021-03-23T01:36:32Z

src/sagemaker/workflow/pipeline.py

@@ -75,7 +75,7 @@ def to_request(self) -> RequestType:
    def create(
        self,
        role_arn: str,
-        description: str = None,
+        pipeline_description: str = None,


Any reason to change this beyond aesthetics? It will be a breaking change, and seems like there is precedence for the prior convention.

docstring parameter used is pipeline_description. I changed it to match it. This is not a breaking change. It passes all unit test

Then the docstring needs to be changed. If someone is passing this with named arguments (pipeline.create(description='My description', ...)) they will get create() got an unexpected keyword argument 'description'. The unit tests pass because they're all using positional arguments.

oh right forgot about that. I'll change the docstring

staubhp · 2021-03-23T01:47:56Z

tests/unit/sagemaker/workflow/test_steps.py

@@ -127,22 +139,26 @@ def test_training_step(sagemaker_session):
        "Type": "Training",
        "Arguments": {
            "AlgorithmSpecification": {"TrainingImage": IMAGE_URI, "TrainingInputMode": "File"},
+            "HyperParameters": {
+                "batch-size": training_batch_size_parameter,


I wouldn't expect to see objects in this assertion. Does it work if asserted on {"Get": "Parameters.TrainingBatchSize"}?

No that won't work. pipeline.to_request() won't interpolate parameters like that but pipeline.definition() would. Here's the code that does that https://github.com/aws/sagemaker-python-sdk/blob/master/src/sagemaker/workflow/pipeline.py#L248-L258

We have older unit tests that perform similar assertions https://github.com/aws/sagemaker-python-sdk/blob/master/tests/unit/sagemaker/workflow/test_pipeline.py#L187.

Lmk if you think I should change pipeline.to_request() logic to return parameter expressions instead of objects.

That's fine, I just wanted to make sure we have coverage on the conversion of those Parameter objects to the JSON code, looks like it's in test_pipeline.py

sagemaker-bot · 2021-03-23T19:56:22Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-23T20:22:06Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-23T20:23:58Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-23T20:33:19Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-23T20:35:05Z

AWS CodeBuild CI Report

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

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

…ters

sagemaker-bot · 2021-03-24T17:51:19Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-24T18:18:48Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-24T18:19:19Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-24T18:28:16Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-24T19:40:56Z

AWS CodeBuild CI Report

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

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

…ters

sagemaker-bot · 2021-03-25T19:09:13Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-25T19:41:43Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-25T19:43:38Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-25T19:48:09Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-25T19:52:55Z

AWS CodeBuild CI Report

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

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

…ters

sagemaker-bot · 2021-03-25T21:16:58Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-25T21:42:32Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-25T21:48:03Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-25T21:48:21Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-03-25T21:49:13Z

AWS CodeBuild CI Report

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

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

…ws#2115) (aws#2227) Co-authored-by: Namrata Madan <[email protected]> Co-authored-by: Ahsan Khan <[email protected]> Co-authored-by: icywang86rui <[email protected]>

staubhp reviewed Mar 23, 2021

View reviewed changes

fix: use workflow parameters in training hyperparameters (#2114) (#2115)

6419e4a

danabens requested a review from a team March 23, 2021 23:29

Merge branch 'master' into fix-use-workflow-parameters-in-hyperparame…

d17c0d9

…ters

staubhp approved these changes Mar 25, 2021

View reviewed changes

Merge branch 'master' into fix-use-workflow-parameters-in-hyperparame…

e8d37d6

…ters

icywang86rui approved these changes Mar 25, 2021

View reviewed changes

Merge branch 'master' into fix-use-workflow-parameters-in-hyperparame…

1a67dbc

…ters

ahsan-z-khan merged commit 1c50ffa into aws:master Mar 25, 2021

fix: use workflow parameters in training hyperparameters (#2114) (#2115) #2227

fix: use workflow parameters in training hyperparameters (#2114) (#2115) #2227

Uh oh!

Conversation

nmadan commented Mar 19, 2021

Merge Checklist

General

Tests

Uh oh!

sagemaker-bot commented Mar 19, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 19, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 19, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 19, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 19, 2021

AWS CodeBuild CI Report

Uh oh!

staubhp Mar 23, 2021

Choose a reason for hiding this comment

Uh oh!

nmadan Mar 23, 2021 • 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!

staubhp Mar 23, 2021

Choose a reason for hiding this comment

Uh oh!

nmadan Mar 23, 2021

Choose a reason for hiding this comment

Uh oh!

staubhp Mar 23, 2021

Choose a reason for hiding this comment

Uh oh!

nmadan Mar 23, 2021 • 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!

staubhp Mar 23, 2021

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Mar 23, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 23, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 23, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 23, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 23, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 24, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 24, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 24, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 24, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 24, 2021

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 25, 2021

AWS CodeBuild CI Report

Uh oh!

nmadan Mar 23, 2021 •

edited

Loading

nmadan Mar 23, 2021 •

edited

Loading