feat: jumpstart model artifact instance type variants #4172

evakravi · 2023-10-06T16:31:04Z

Issue #, if available:

Description of changes:
JumpStart plans on introducing models that may have different model artifacts depending on the instance type chosen. This will require new metadata, and new SDK code for consuming this metadata.

Testing done:
Unit tests added

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 backward 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 · 2023-10-06T17:27:54Z

AWS CodeBuild CI Report

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

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

whittech1 · 2023-10-06T17:21:55Z

src/sagemaker/jumpstart/types.py

+        """
+
+        return self._get_instance_specific_property(
+            instance_type=instance_type, property_name="prepacked_artifact_key"


nit: should the property name include s3?

whittech1 · 2023-10-06T17:24:16Z

src/sagemaker/jumpstart/artifacts/model_uris.py

+                instance_type=instance_type
+            )
+            if instance_type
+            and getattr(model_specs, "hosting_instance_type_variants", None) is not None


Isn't the name of the property hosting_custom_instance_type_variants in the MH PR?

whittech1 · 2023-10-06T17:33:21Z

src/sagemaker/jumpstart/artifacts/model_uris.py

        model_artifact_key = (
-            getattr(model_specs, "hosting_prepacked_artifact_key", None)
-            or model_specs.hosting_artifact_key
+            instance_specific_prepacked_hosting_artifact_key


I think this logic can break if there is an instance-specific normal artifact but a default prepacked artifact. Previously, just the presence of default prepacked artifact would have indicated a need to prepack.

To be exact, we need to change the execution flow based on whether model_specs is prepacked or not. Quick-and-dirty approach would be something like:

is_prepacked = model_specs.hosting_use_script_uri or default_prepacked_hosting_artifact_key if is_prepacked: return instance_specific_prepack_key or default_prepacked_hosting_artifact_key else: return instance_specific_key or default_hosting_artifact_key

sagemaker-bot · 2023-10-06T17:36:05Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-06T17:37:29Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-06T18:22:30Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-06T19:15:21Z

AWS CodeBuild CI Report

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

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

JGuinegagne

Could you add specific unit tests for (a) sagemaker/model_uri and (b) ssagemaker/jumpstart/artifacts/model_uris please?

I know you are implicitely testing through JumpStartModel and JumpStartEstimator, but we should guard ourselves against specific logic introduced in these classes and test the utilities directly as well.

JGuinegagne · 2023-10-09T13:13:46Z

src/sagemaker/jumpstart/artifacts/model_uris.py

@@ -50,6 +51,8 @@ def _retrieve_model_uri(
            artifact S3 URI.
        model_scope (str): The model type, i.e. what it is used for.
            Valid values: "training" and "inference".
+        instance_type (str): An instance type to optionally supply in order to get
+            model artifacts specific for the instance type. (Default: None).


Optional. The ML compute instance type for the specified scope. (Default: None).

src/sagemaker/jumpstart/artifacts/model_uris.py

JGuinegagne · 2023-10-09T13:26:59Z

src/sagemaker/model_uris.py

@@ -44,6 +45,8 @@ def retrieve(
            the model artifact S3 URI.
        model_scope (str): The model type.
            Valid values: "training" and "inference".
+        instance_type (str): An instance type to optionally supply in order to get
+            model artifacts specific for the instance type. (Default: None).


same comment here.

JGuinegagne · 2023-10-09T13:29:05Z

tests/unit/sagemaker/jumpstart/constants.py

+                    "properties": {"environment_variables": {"TENSOR_PARALLEL_DEGREE": "8"}}
+                },
+                "ml.g5.12xlarge": {
+                    "properties": {"environment_variables": {"TENSOR_PARALLEL_DEGREE": "4"}}


could you add a test with an instance specific prepacked_artifact_key as well please?

As in not just instance family variant please.

Same for training case please

sagemaker-bot · 2023-10-09T16:32:46Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-09T16:38:28Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-09T16:45:26Z

AWS CodeBuild CI Report

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

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

JGuinegagne

Could you update the docstring as well please?

sagemaker-bot · 2023-10-09T18:17:27Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-09T18:24:35Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-09T18:27:27Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-09T18:56:37Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-09T19:10:03Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-10T00:15:19Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-10T00:22:41Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-10T00:30:18Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-10T00:30:50Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-10T01:08:10Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-13T19:39:08Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-13T19:48:36Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-13T20:25:50Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-13T21:15:17Z

AWS CodeBuild CI Report

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

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

…act-instance-type-variants

sagemaker-bot · 2023-10-16T17:29:24Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-16T17:35:52Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-16T18:21:07Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-16T18:21:53Z

AWS CodeBuild CI Report

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

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

evakravi

/bot run slow-tests

sagemaker-bot · 2023-10-16T20:47:43Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-17T00:33:31Z

AWS CodeBuild CI Report

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

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

evakravi

/bot run pr

sagemaker-bot · 2023-10-17T15:30:45Z

AWS CodeBuild CI Report

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

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

knikure

/bot run pr

sagemaker-bot · 2023-10-17T17:39:20Z

AWS CodeBuild CI Report

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

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

knikure · 2023-10-17T20:31:43Z

@evakravi Can you resolve conflicts on this PR?

…act-instance-type-variants

sagemaker-bot · 2023-10-18T00:34:12Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-18T00:36:25Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-18T01:20:06Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2023-10-18T01:28:39Z

AWS CodeBuild CI Report

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

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

codecov-commenter · 2023-10-18T01:29:07Z

Codecov Report

Attention: 45 lines in your changes are missing coverage. Please review.

Comparison is base (3057f21) 90.27% compared to head (94fedaa) 90.33%.
Report is 38 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4172      +/-   ##
==========================================
+ Coverage   90.27%   90.33%   +0.06%     
==========================================
  Files         993      320     -673     
  Lines       87861    29586   -58275     
==========================================
- Hits        79313    26726   -52587     
+ Misses       8548     2860    -5688

Files	Coverage Δ
src/sagemaker/base_predictor.py	`90.90% <100.00%> (ø)`
src/sagemaker/config/config_schema.py	`100.00% <100.00%> (ø)`
src/sagemaker/estimator.py	`92.88% <ø> (ø)`
src/sagemaker/feature_store/feature_group.py	`78.82% <ø> (ø)`
...emaker/feature_store/feature_processor/__init__.py	`100.00% <100.00%> (ø)`
...eature_store/feature_processor/_config_uploader.py	`97.43% <100.00%> (ø)`
...aker/feature_store/feature_processor/_constants.py	`100.00% <100.00%> (ø)`
...er/feature_store/feature_processor/_data_source.py	`100.00% <100.00%> (ø)`
...agemaker/feature_store/feature_processor/_enums.py	`100.00% <100.00%> (ø)`
...ore/feature_processor/_feature_processor_config.py	`100.00% <100.00%> (ø)`
... and 47 more

... and 1256 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sagemaker-bot · 2023-10-18T01:32:18Z

AWS CodeBuild CI Report

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

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

evakravi requested a review from a team as a code owner October 6, 2023 16:31

evakravi requested review from vpbhargav and removed request for a team October 6, 2023 16:31

evakravi marked this pull request as draft October 6, 2023 16:32

whittech1 reviewed Oct 6, 2023

View reviewed changes

JGuinegagne requested changes Oct 9, 2023

View reviewed changes

JGuinegagne reviewed Oct 9, 2023

View reviewed changes

evakravi added 3 commits October 9, 2023 16:19

feat: jumpstart model artifact instance type variants

ce47702

chore: address PR comments

1c72c74

chore: improve docstring

11e20e5

goelakash force-pushed the feat/jumpstart-model-artifact-instance-type-variants branch from a54ab0a to 11e20e5 Compare October 9, 2023 23:19

evakravi marked this pull request as ready for review October 13, 2023 17:31

Merge remote-tracking branch 'origin' into feat/jumpstart-model-artif…

195ceee

…act-instance-type-variants

whittech1 approved these changes Oct 16, 2023

View reviewed changes

JGuinegagne approved these changes Oct 16, 2023

View reviewed changes

evakravi commented Oct 16, 2023

View reviewed changes

evakravi commented Oct 17, 2023

View reviewed changes

knikure reviewed Oct 17, 2023

View reviewed changes

knikure self-assigned this Oct 17, 2023

Merge remote-tracking branch 'origin' into feat/jumpstart-model-artif…

94fedaa

…act-instance-type-variants

knikure merged commit dba2dbb into aws:master Oct 18, 2023

knikure pushed a commit to knikure/sagemaker-python-sdk that referenced this pull request Oct 23, 2023

feat: jumpstart model artifact instance type variants (aws#4172)

3f6cfe8

benieric pushed a commit to benieric/sagemaker-python-sdk that referenced this pull request Oct 24, 2023

feat: jumpstart model artifact instance type variants (aws#4172)

e62e8ba

feat: jumpstart model artifact instance type variants #4172

feat: jumpstart model artifact instance type variants #4172

Uh oh!

Conversation

evakravi commented Oct 6, 2023

Merge Checklist

General

Tests

Uh oh!

sagemaker-bot commented Oct 6, 2023

AWS CodeBuild CI Report

Uh oh!

whittech1 Oct 6, 2023

Choose a reason for hiding this comment

Uh oh!

whittech1 Oct 6, 2023

Choose a reason for hiding this comment

Uh oh!

whittech1 Oct 6, 2023

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Oct 6, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 6, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 6, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 6, 2023

AWS CodeBuild CI Report

Uh oh!

JGuinegagne left a comment

Choose a reason for hiding this comment

Uh oh!

JGuinegagne Oct 9, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JGuinegagne Oct 9, 2023

Choose a reason for hiding this comment

Uh oh!

JGuinegagne Oct 9, 2023

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Oct 9, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 9, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 9, 2023

AWS CodeBuild CI Report

Uh oh!

JGuinegagne left a comment

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Oct 9, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 9, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 9, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 9, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 9, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 10, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 10, 2023

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Oct 10, 2023