change: use recommended inference image URI from Neo API #2806

HappyAmazonian · 2021-12-20T21:27:40Z

Issue #, if available:

Description of changes:

use recommended inference image URI from Neo DescribeCompilationJob API response
related unit tests

Testing done:
unit tests passed

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.

aws#2790)

…ries (aws#2755) Co-authored-by: Payton Staub <[email protected]> Co-authored-by: Ahsan Khan <[email protected]>

Co-authored-by: Shreya Pandit <[email protected]>

Co-authored-by: Basil Beirouti <[email protected]> Co-authored-by: Payton Staub <[email protected]> Co-authored-by: Ahsan Khan <[email protected]> Co-authored-by: Mufaddal Rohawala <[email protected]> Co-authored-by: Basil Beirouti <[email protected]> Co-authored-by: Payton Staub <[email protected]> Co-authored-by: Shreya Pandit <[email protected]>

codecov-commenter · 2021-12-20T21:40:19Z

Codecov Report

Merging #2806 (13aa858) into dev (c2b5f95) will increase coverage by 0.21%.
The diff coverage is 95.08%.

@@            Coverage Diff             @@
##              dev    #2806      +/-   ##
==========================================
+ Coverage   88.71%   88.93%   +0.21%     
==========================================
  Files         175      178       +3     
  Lines       15500    15755     +255     
==========================================
+ Hits        13751    14011     +260     
+ Misses       1749     1744       -5

Impacted Files	Coverage Δ
src/sagemaker/image_uris.py	`99.32% <ø> (ø)`
src/sagemaker/tensorflow/model.py	`89.13% <ø> (ø)`
src/sagemaker/workflow/execution_variables.py	`100.00% <ø> (ø)`
src/sagemaker/lineage/action.py	`85.91% <58.33%> (-14.09%)`	⬇️
src/sagemaker/tuner.py	`97.91% <83.33%> (-0.22%)`	⬇️
src/sagemaker/lineage/context.py	`81.98% <94.73%> (+6.63%)`	⬆️
src/sagemaker/workflow/properties.py	`88.15% <94.73%> (+0.83%)`	⬆️
src/sagemaker/lineage/query.py	`94.96% <97.61%> (+14.80%)`	⬆️
src/sagemaker/clarify.py	`92.91% <100.00%> (+0.51%)`	⬆️
src/sagemaker/estimator.py	`91.18% <100.00%> (+0.58%)`	⬆️
... and 16 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 c2b5f95...13aa858. Read the comment docs.

sagemaker-bot · 2021-12-20T21:47:53Z

AWS CodeBuild CI Report

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

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

HappyAmazonian · 2021-12-21T19:22:36Z

Hi, is it possible to check the log for those failed tests?

src/sagemaker/model.py

shreyapandit

Minor comment to be discussed

sagemaker-bot · 2021-12-21T23:15:19Z

AWS CodeBuild CI Report

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

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

src/sagemaker/model.py

navinsoni · 2021-12-22T22:37:01Z

tests/unit/sagemaker/model/test_neo.py

@@ -286,7 +282,7 @@ def test_compile_with_framework_version_15(session):
        job_name="compile-model",
    )

-    assert "1.5" in model.image_uri
+    assert model.image_uri is not None


any specific reason we are not checking "1.5" here in image uri

Since the uri is fetched from Neo API now, I think it would be suffice to just check that
model.image_uri is equal to the uri inside the response.

I think we should still check for 1.5 to make sure the uri we are receiving is as expected

I don't think that is possible. Checking 1.5 requires API access within the unit tests. I changed it to make sure the model.image_uri is the same as what is inside the response.

navinsoni · 2021-12-22T22:37:14Z

tests/unit/sagemaker/model/test_neo.py

-        )
-
-    assert "Unsupported neo-pytorch version: 1.6.1." in str(e)
+    assert model.image_uri is not None


navinsoni · 2021-12-22T22:38:01Z

tests/unit/sagemaker/model/test_neo.py

-@patch("sagemaker.session.Session")
-def test_compile_validates_framework_version(session):
-    session.return_value.boto_region_name = REGION
-
-    model = _create_model()
-    with pytest.raises(ValueError) as e:
-        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.1",
-            job_name="compile-model",
-        )
-
-    assert "Unsupported neo-pytorch version: 1.6.1." in str(e)


any specific reason we are removing negative test case

Same as the previous reply.
Since the uri is fetched from Neo API now, the API response will put inference uri as None if there is not a proper inference image.

…nse to get

…emaker-python-sdk into inference-url

sagemaker-bot · 2021-12-27T19:22:29Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-12-30T00:50:13Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2021-12-30T01:24:24Z

AWS CodeBuild CI Report

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

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

* update smdmp change log, archive api doc for 1.4.0 and 1.5.0 * add no-index flags * finish api doc archive * fix: Set ProcessingStep upload locations deterministically to avoid c… (aws#2790) * fix: Prevent repack_model script from referencing nonexistent directories (aws#2755) Co-authored-by: Payton Staub <[email protected]> Co-authored-by: Ahsan Khan <[email protected]> * fix: S3Input - add support for instance attributes (aws#2754) * fix: typos and broken link (aws#2765) Co-authored-by: Shreya Pandit <[email protected]> * add all api docs * add appendix, fix links * structural changes, fix links * incorporate feedback * prepare release v2.72.1 * update development version to v2.72.2.dev0 Co-authored-by: Payton Staub <[email protected]> Co-authored-by: Payton Staub <[email protected]> Co-authored-by: Ahsan Khan <[email protected]> Co-authored-by: Mufaddal Rohawala <[email protected]> Co-authored-by: Mohamed Ali Jamaoui <[email protected]> Co-authored-by: Shreya Pandit <[email protected]> Co-authored-by: ci <ci> Co-authored-by: Jeniya Tabassum <[email protected]>

…s#2815)

sagemaker-bot · 2021-12-31T21:20:22Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-01-11T19:32:23Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-01-11T19:35:03Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-01-11T19:49:30Z

AWS CodeBuild CI Report

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

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

shreyapandit · 2022-01-14T22:28:03Z

src/sagemaker/model.py

-                    framework,
-                    framework_version,
-                )
+                self.image_uri = job_status.get("InferenceImage", None)


What is the impact if this is None? Would that prevent creation of model? Can we have a default instead?

What is the impact if this is None?
It would be None only if there is no recommended inference image. In that case, all other functions depending on this value ought to fail naturally since it is not valid.

Would that prevent creation of model?
No. In model.compile function, it simply launches a compilation job.

Can we have a default instead?
From now on, we want user to use the image_uri provided by Neo service. Even if we decide to have a default image-uri later, we will make that change through our API response.

…ts (aws#2838)

Co-authored-by: chenxy <[email protected]>

Currently the Clarify BiasConfig only accepts facet name. Actually Clarify analysis configuration supports both name and index. This commit adds the same support to BiasConfig.

sagemaker-bot · 2022-01-21T20:46:45Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-01-21T21:41:44Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-01-21T21:51:03Z

AWS CodeBuild CI Report

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

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

HappyAmazonian · 2022-02-09T20:02:54Z

Since the history and the diff are getting messier overtime, I create a new PR if that helps.

HappyAmazonian · 2022-02-09T20:11:05Z

Since the history and the diff are getting messier overtime, I create a new #2923 if that helps.

staubhp and others added 6 commits December 15, 2021 10:45

fix: Set ProcessingStep upload locations deterministically to avoid c… (

dde8d00

aws#2790)

fix: Prevent repack_model script from referencing nonexistent directo…

0f72907

…ries (aws#2755) Co-authored-by: Payton Staub <[email protected]> Co-authored-by: Ahsan Khan <[email protected]>

fix: S3Input - add support for instance attributes (aws#2754)

0bae071

fix: typos and broken link (aws#2765)

17fe93e

Co-authored-by: Shreya Pandit <[email protected]>

change: use recommended inference image uri from Neo API

08c3a7b

shreyapandit reviewed Dec 21, 2021

View reviewed changes

src/sagemaker/model.py Outdated Show resolved Hide resolved

shreyapandit suggested changes Dec 21, 2021

View reviewed changes

Merge branch 'master' into inference-url

1d5a5dc

navinsoni reviewed Dec 22, 2021

View reviewed changes

HappyAmazonian added 2 commits December 27, 2021 18:56

change InferenceImage lookup in compilation job description API respo…

edbf61f

…nse to get

Merge branch 'inference-url' of https://github.com/HappyAmazonian/sag…

d28b89f

…emaker-python-sdk into inference-url

navinns and others added 2 commits December 29, 2021 16:21

fix: Add ContentType in test_auto_ml_describe

185b4b0

fix: Re-deploy static integ test endpoint if it is not found

5ec2ff4

mufaddal-rohawala changed the base branch from master to dev December 30, 2021 00:34

Merge branch 'dev' into inference-url

cec05a5

mchoi8739 and others added 3 commits December 30, 2021 07:30

fix: fix kmeans test deletion sequence, increment lineage statics (aw…

6319bce

…s#2815)

Merge branch 'dev' into inference-url

f728e11

mufaddal-rohawala requested review from navinsoni and shreyapandit January 3, 2022 19:35

fix: Increment static lineage pipeline (aws#2817)

beeabbd

shreyapandit suggested changes Jan 14, 2022

View reviewed changes

shreyapandit added the needs contributor changes label Jan 14, 2022

bhaoz and others added 4 commits January 18, 2022 12:05

feature: Adds support for Serverless inference (aws#2831)

81af309

feature: Add support for SageMaker lineage queries in action (aws#2853)

a949e5f

feature: Adds Lineage queries in artifact, context and trial componen…

888153b

…ts (aws#2838)

feature: Add EMRStep support in Sagemaker pipeline (aws#2848)

b796a00

Co-authored-by: chenxy <[email protected]>

shreyapandit force-pushed the dev branch from 903205c to b796a00 Compare January 18, 2022 20:09

yzhu0 and others added 4 commits January 19, 2022 13:08

feature: Add support for SageMaker lineage queries context (aws#2830)

9d25fcf

fix: support specifying a facet by its column index

64e4690

Currently the Clarify BiasConfig only accepts facet name. Actually Clarify analysis configuration supports both name and index. This commit adds the same support to BiasConfig.

doc: more documentation for serverless inference (aws#2859)

814dd3d

Merge branch 'dev' into inference-url

13aa858

BasilBeirouti force-pushed the dev branch from 814dd3d to 70308b1 Compare January 24, 2022 20:10

jeniyat force-pushed the dev branch 3 times, most recently from f5e6849 to 34b07c0 Compare January 29, 2022 23:28

mufaddal-rohawala force-pushed the dev branch 3 times, most recently from 96c01d7 to c437191 Compare February 5, 2022 00:08

jeniyat force-pushed the dev branch from 9d612a7 to bb7563f Compare February 7, 2022 23:40

HappyAmazonian requested a review from shreyapandit February 9, 2022 18:03

HappyAmazonian mentioned this pull request Feb 9, 2022

change: use recommended inference image uri from Neo API #2923

Merged

9 tasks

HappyAmazonian closed this Feb 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

change: use recommended inference image URI from Neo API #2806

change: use recommended inference image URI from Neo API #2806

HappyAmazonian commented Dec 20, 2021 •

edited

Loading

codecov-commenter commented Dec 20, 2021 •

edited

Loading

sagemaker-bot commented Dec 20, 2021

HappyAmazonian commented Dec 21, 2021

shreyapandit left a comment

sagemaker-bot commented Dec 21, 2021

navinsoni Dec 22, 2021

HappyAmazonian Dec 22, 2021

navinsoni Jan 3, 2022

HappyAmazonian Jan 4, 2022

navinsoni Dec 22, 2021

navinsoni Dec 22, 2021

HappyAmazonian Dec 27, 2021

sagemaker-bot commented Dec 27, 2021

sagemaker-bot commented Dec 30, 2021

sagemaker-bot commented Dec 30, 2021

sagemaker-bot commented Dec 31, 2021

sagemaker-bot commented Jan 11, 2022

sagemaker-bot commented Jan 11, 2022

sagemaker-bot commented Jan 11, 2022

shreyapandit Jan 14, 2022

HappyAmazonian Jan 14, 2022

sagemaker-bot commented Jan 21, 2022

sagemaker-bot commented Jan 21, 2022

sagemaker-bot commented Jan 21, 2022

HappyAmazonian commented Feb 9, 2022 •

edited

Loading

HappyAmazonian commented Feb 9, 2022

change: use recommended inference image URI from Neo API #2806

change: use recommended inference image URI from Neo API #2806

Conversation

HappyAmazonian commented Dec 20, 2021 • edited Loading

Merge Checklist

General

Tests

codecov-commenter commented Dec 20, 2021 • edited Loading

Codecov Report

sagemaker-bot commented Dec 20, 2021

AWS CodeBuild CI Report

HappyAmazonian commented Dec 21, 2021

shreyapandit left a comment

Choose a reason for hiding this comment

sagemaker-bot commented Dec 21, 2021

AWS CodeBuild CI Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sagemaker-bot commented Dec 27, 2021

AWS CodeBuild CI Report

sagemaker-bot commented Dec 30, 2021

AWS CodeBuild CI Report

sagemaker-bot commented Dec 30, 2021

AWS CodeBuild CI Report

sagemaker-bot commented Dec 31, 2021

AWS CodeBuild CI Report

sagemaker-bot commented Jan 11, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Jan 11, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Jan 11, 2022

AWS CodeBuild CI Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sagemaker-bot commented Jan 21, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Jan 21, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Jan 21, 2022

AWS CodeBuild CI Report

HappyAmazonian commented Feb 9, 2022 • edited Loading

HappyAmazonian commented Feb 9, 2022

HappyAmazonian commented Dec 20, 2021 •

edited

Loading

codecov-commenter commented Dec 20, 2021 •

edited

Loading

HappyAmazonian commented Feb 9, 2022 •

edited

Loading