fix: estimator hyperparameters in script mode #3344

evakravi · 2022-09-02T18:14:26Z

Issue #, if available:

Description of changes:
Previously, when the Estimator class is used in script mode (source_dir is supplied), the hyperparameters added by the user would not be escaped as Json. This introduced a discrepancy between how the Estimator and Frameworkclasses handled hyperparameters. This discrepancy didn't cause any negative customer impact (failed jobs).

Testing done:

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

I have read the CONTRIBUTING doc
I certify that the changes I am introducing will be backword compatible, and I have discussed concerns about this, if any, with the Python SDK team
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

I have added tests that prove my fix is effective or that my feature works (if appropriate)
I have added unit and/or integration tests as appropriate to ensure backward compatibility of the changes
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 · 2022-09-02T18:44:37Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-02T18:50:38Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-02T19:00:14Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-02T19:18:38Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-02T19:21:34Z

AWS CodeBuild CI Report

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

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

codecov-commenter · 2022-09-02T19:21:37Z

Codecov Report

Merging #3344 (e1b614d) into master (38b4916) will decrease coverage by 0.23%.
The diff coverage is 93.19%.

@@            Coverage Diff             @@
##           master    #3344      +/-   ##
==========================================
- Coverage   89.40%   89.17%   -0.24%     
==========================================
  Files         848      204     -644     
  Lines       72563    18979   -53584     
==========================================
- Hits        64877    16924   -47953     
+ Misses       7686     2055    -5631

Impacted Files	Coverage Δ
src/sagemaker/jumpstart/types.py	`94.37% <0.00%> (ø)`
src/sagemaker/processing.py	`94.33% <ø> (ø)`
...agemaker/serverless/serverless_inference_config.py	`100.00% <ø> (ø)`
src/sagemaker/transformer.py	`94.47% <ø> (ø)`
src/sagemaker/workflow/step_collections.py	`93.87% <ø> (ø)`
src/sagemaker/workflow/conditions.py	`78.57% <50.00%> (-2.08%)`	⬇️
src/sagemaker/session.py	`71.66% <68.42%> (-0.16%)`	⬇️
src/sagemaker/huggingface/estimator.py	`94.44% <75.00%> (+1.42%)`	⬆️
src/sagemaker/job.py	`90.00% <75.00%> (-0.55%)`	⬇️
src/sagemaker/tensorflow/estimator.py	`92.18% <75.00%> (-0.56%)`	⬇️
... and 727 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

sagemaker-bot · 2022-09-06T06:19:14Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-06T06:24:13Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-06T06:36:03Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-06T06:55:28Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-06T08:41:21Z

AWS CodeBuild CI Report

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

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

…nit test

sagemaker-bot · 2022-09-06T12:59:49Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-06T13:25:27Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-06T13:28:57Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-06T13:46:59Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-06T14:03:10Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-07T14:17:07Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-07T14:36:21Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-07T14:52:50Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-07T15:13:09Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-07T15:17:44Z

AWS CodeBuild CI Report

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

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

mufaddal-rohawala · 2022-09-07T16:10:47Z

src/sagemaker/estimator.py

        """
-        for k, v in kwargs.items():
-            self._hyperparameters[k] = v
+        if self.source_dir:


is this a strict contract between source_dir and use of _json_encode_hyperparameters?

yes. If we always encode the hyper parameters, some unit tests will fail.

sagemaker-bot · 2022-09-08T15:19:54Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-08T15:23:41Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-08T15:39:25Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-08T15:57:19Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-08T17:42:19Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-08T21:33:14Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-09T11:27:53Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-09T11:48:26Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-09T12:04:30Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-09T12:20:08Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-09T12:22:25Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-12T17:16:26Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-12T17:35:39Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-12T17:55:30Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-12T18:18:05Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-12T18:22:02Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-09-12T18:45:42Z

AWS CodeBuild CI Report

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

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

fix: estimator hyperparameters in script mode

6c9b085

JGuinegagne approved these changes Sep 6, 2022

View reviewed changes

evakravi and others added 2 commits September 6, 2022 12:53

chore: ensure parity between estimator and framework hyperparams in u…

da64c38

…nit test

Merge branch 'master' into fix/estimator-hyperparameters

8b6edc8

Merge branch 'master' into fix/estimator-hyperparameters

b9d0033

mufaddal-rohawala reviewed Sep 7, 2022

View reviewed changes

Merge branch 'master' into fix/estimator-hyperparameters

4a6912f

Merge branch 'master' into fix/estimator-hyperparameters

263fe22

Merge branch 'master' into fix/estimator-hyperparameters

e1b614d

mufaddal-rohawala approved these changes Sep 12, 2022

View reviewed changes

mufaddal-rohawala merged commit ace07d7 into aws:master Sep 12, 2022

maekawataiki mentioned this pull request Nov 25, 2022

hyperparameters.json hyperparameter encoding breaks backward compatibility #3487

Closed

JoseJuan98 pushed a commit to JoseJuan98/sagemaker-python-sdk that referenced this pull request Mar 4, 2023

fix: estimator hyperparameters in script mode (aws#3344)

6a55493

fix: estimator hyperparameters in script mode #3344

fix: estimator hyperparameters in script mode #3344

Conversation

evakravi commented Sep 2, 2022

Merge Checklist

General

Tests

sagemaker-bot commented Sep 2, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 2, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 2, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 2, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 2, 2022

AWS CodeBuild CI Report

codecov-commenter commented Sep 2, 2022 • edited Loading

Codecov Report

sagemaker-bot commented Sep 6, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 6, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 6, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 6, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 6, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 6, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 6, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 6, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 6, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 6, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 7, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 7, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 7, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 7, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 7, 2022

AWS CodeBuild CI Report

mufaddal-rohawala Sep 7, 2022

Choose a reason for hiding this comment

evakravi Sep 12, 2022

Choose a reason for hiding this comment

sagemaker-bot commented Sep 8, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 8, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 8, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 8, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 8, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 8, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 9, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 9, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 9, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 9, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 9, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 12, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 12, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Sep 12, 2022

codecov-commenter commented Sep 2, 2022 •

edited

Loading