fix: make 'ModelInput' field optional for inference recommendation #3220

rahven14 · 2022-07-08T18:25:31Z

Issue #, if available: N/A

Description of changes: Make 'ModelInput' field optional for inference recommendation and refactor code for better readability.

Testing done: units

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-07-08T18:52:27Z

AWS CodeBuild CI Report

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

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

codecov-commenter · 2022-07-08T18:52:50Z

Codecov Report

Merging #3220 (e51e492) into master (9369a87) will decrease coverage by 0.61%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3220      +/-   ##
==========================================
- Coverage   89.46%   88.85%   -0.62%     
==========================================
  Files         848      203     -645     
  Lines       72426    17950   -54476     
==========================================
- Hits        64796    15949   -48847     
+ Misses       7630     2001    -5629

Impacted Files	Coverage Δ
src/sagemaker/utils.py	`92.51% <100.00%> (+0.11%)`	⬆️
...on3.8/site-packages/sagemaker/debugger/__init__.py
...ib/python3.7/site-packages/sagemaker/exceptions.py
...thon3.7/site-packages/sagemaker/lineage/context.py
...3.7/site-packages/sagemaker/wrangler/processing.py
...3.7/site-packages/sagemaker/jumpstart/accessors.py
...b/python3.9/site-packages/sagemaker/local/utils.py
...python3.8/site-packages/sagemaker/lineage/query.py
...agemaker/serverless/serverless_inference_config.py
...on3.8/site-packages/sagemaker/sklearn/estimator.py
... and 636 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9369a87...e51e492. Read the comment docs.

sagemaker-bot · 2022-07-08T19:01:23Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-08T19:14:05Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-08T19:21:10Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-08T19:33:12Z

AWS CodeBuild CI Report

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

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

saumitravikram · 2022-07-11T06:28:13Z

src/sagemaker/utils.py

+    if data_input_configuration is not None:
+        obj.update(
+            {
+                "Framework": framework,
+                "FrameworkVersion": framework_version,
+                "NearestModelName": nearest_model_name,
+                "ModelInput": {
+                    "DataInputConfig": data_input_configuration,
+                },
+            }
+        )
+    else:
+        obj.update(
+            {
+                "Framework": framework,
+                "FrameworkVersion": framework_version,
+                "NearestModelName": nearest_model_name,
+            }
+        )


nit : Can the obj be partially updated also? Then we can refactor the code saying update the 3 fields(Framework, FrameworkVersion, NearestModelName) first and then if data_input_configuration is not none then update ModelInput

Yes, this can be refactored. Thank you for pointing this out!

saumitravikram · 2022-07-11T06:29:29Z

tests/unit/test_estimator.py

@@ -3319,7 +3317,6 @@ def test_register_inference_image(sagemaker_session):
    framework = "TENSORFLOW"
    framework_version = "2.9"
    nearest_model_name = "resnet50"
-    data_input_config = '{"input_1":[1,224,224,3]}'


Have atleast 1 new test where we check for data_input_config if not already present.

It is already present in the existing test files.

…hub.com/rahven14/sagemaker-python-sdk into fix/update-inference-recommender-fields

sagemaker-bot · 2022-07-11T10:22:42Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-11T10:23:23Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-11T10:32:30Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-11T10:42:04Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-11T10:46:57Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-11T11:02:05Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-11T11:03:39Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-11T12:48:22Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-11T12:50:13Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-12T04:43:51Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-07-12T06:21:02Z

AWS CodeBuild CI Report

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

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

…ws#3220) * fix: make 'ModelInput' field optional for inference recommendation * fix: refactor code to conditionally update container object

fix: make 'ModelInput' field optional for inference recommendation

1f3fbe7

saumitravikram reviewed Jul 11, 2022

View reviewed changes

rahven14 and others added 3 commits July 11, 2022 15:25

Merge branch 'aws:master' into fix/update-inference-recommender-fields

037a574

fix: refactor code to conditionally update container object

f536d35

Merge branch 'fix/update-inference-recommender-fields' of https://git…

e51e492

…hub.com/rahven14/sagemaker-python-sdk into fix/update-inference-recommender-fields

saumitravikram approved these changes Jul 12, 2022

View reviewed changes

mufaddal-rohawala approved these changes Jul 12, 2022

View reviewed changes

shreyapandit approved these changes Jul 12, 2022

View reviewed changes

shreyapandit merged commit c0929cc into aws:master Jul 12, 2022

fix: make 'ModelInput' field optional for inference recommendation #3220

fix: make 'ModelInput' field optional for inference recommendation #3220

Uh oh!

Conversation

rahven14 commented Jul 8, 2022

Merge Checklist

General

Tests

Uh oh!

sagemaker-bot commented Jul 8, 2022

AWS CodeBuild CI Report

Uh oh!

codecov-commenter commented Jul 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sagemaker-bot commented Jul 8, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 8, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 8, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 8, 2022

AWS CodeBuild CI Report

Uh oh!

saumitravikram Jul 11, 2022

Choose a reason for hiding this comment

Uh oh!

rahven14 Jul 11, 2022

Choose a reason for hiding this comment

Uh oh!

saumitravikram Jul 11, 2022

Choose a reason for hiding this comment

Uh oh!

rahven14 Jul 11, 2022

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Jul 11, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 11, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 11, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 11, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 11, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 11, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 11, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 11, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 11, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 12, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 12, 2022

AWS CodeBuild CI Report

Uh oh!

Uh oh!

codecov-commenter commented Jul 8, 2022 •

edited

Loading