From a87f9bd0889727cfe94b1e151266bd7b6e874b31 Mon Sep 17 00:00:00 2001 From: Rahul Venkatesh Date: Tue, 12 Jul 2022 14:11:41 +0530 Subject: [PATCH 1/4] fix: enable model.register without 'inference_instances' & 'transform_instances' --- src/sagemaker/session.py | 14 +- src/sagemaker/workflow/_utils.py | 9 +- .../test_model_create_and_registration.py | 247 ++++++++++++++++++ .../workflow/test_pipeline_session.py | 64 ++++- tests/unit/test_session.py | 66 ++++- 5 files changed, 387 insertions(+), 13 deletions(-) diff --git a/src/sagemaker/session.py b/src/sagemaker/session.py index f426724b6c..e5e9b171e9 100644 --- a/src/sagemaker/session.py +++ b/src/sagemaker/session.py @@ -4499,9 +4499,19 @@ def get_create_model_package_request( "Containers": containers, "SupportedContentTypes": content_types, "SupportedResponseMIMETypes": response_types, - "SupportedRealtimeInferenceInstanceTypes": inference_instances, - "SupportedTransformInstanceTypes": transform_instances, } + if inference_instances is not None: + inference_specification.update( + { + "SupportedRealtimeInferenceInstanceTypes": inference_instances, + } + ) + if transform_instances is not None: + inference_specification.update( + { + "SupportedTransformInstanceTypes": transform_instances, + } + ) request_dict["InferenceSpecification"] = inference_specification request_dict["CertifyForMarketplace"] = marketplace_cert request_dict["ModelApprovalStatus"] = approval_status diff --git a/src/sagemaker/workflow/_utils.py b/src/sagemaker/workflow/_utils.py index 7a0a399299..f8a99996a5 100644 --- a/src/sagemaker/workflow/_utils.py +++ b/src/sagemaker/workflow/_utils.py @@ -341,16 +341,11 @@ def __init__( super(_RegisterModelStep, self).__init__( name, StepTypeEnum.REGISTER_MODEL, display_name, description, depends_on, retry_policies ) - deprecated_args_missing = ( - content_types is None - or response_types is None - or inference_instances is None - or transform_instances is None - ) + deprecated_args_missing = content_types is None or response_types is None if not (step_args is None) ^ deprecated_args_missing: raise ValueError( "step_args and the set of (content_types, response_types, " - "inference_instances, transform_instances) are mutually exclusive. " + ") are mutually exclusive. " "Either of them should be provided." ) diff --git a/tests/integ/sagemaker/workflow/test_model_create_and_registration.py b/tests/integ/sagemaker/workflow/test_model_create_and_registration.py index d0f617a266..63aae962fa 100644 --- a/tests/integ/sagemaker/workflow/test_model_create_and_registration.py +++ b/tests/integ/sagemaker/workflow/test_model_create_and_registration.py @@ -199,6 +199,253 @@ def test_conditional_pytorch_training_model_registration( pass +def test_conditional_pytorch_training_model_registration_without_instance_types( + sagemaker_session, + role, + cpu_instance_type, + pipeline_name, + region_name, +): + base_dir = os.path.join(DATA_DIR, "pytorch_mnist") + entry_point = os.path.join(base_dir, "mnist.py") + input_path = sagemaker_session.upload_data( + path=os.path.join(base_dir, "training"), + key_prefix="integ-test-data/pytorch_mnist/training", + ) + inputs = TrainingInput(s3_data=input_path) + + instance_count = ParameterInteger(name="InstanceCount", default_value=1) + instance_type = "ml.m5.xlarge" + good_enough_input = ParameterInteger(name="GoodEnoughInput", default_value=1) + in_condition_input = ParameterString(name="Foo", default_value="Foo") + + task = "IMAGE_CLASSIFICATION" + sample_payload_url = "s3://test-bucket/model" + framework = "TENSORFLOW" + framework_version = "2.9" + nearest_model_name = "resnet50" + data_input_configuration = '{"input_1":[1,224,224,3]}' + + # If image_uri is not provided, the instance_type should not be a pipeline variable + # since instance_type is used to retrieve image_uri in compile time (PySDK) + pytorch_estimator = PyTorch( + entry_point=entry_point, + role=role, + framework_version="1.5.0", + py_version="py3", + instance_count=instance_count, + instance_type=instance_type, + sagemaker_session=sagemaker_session, + ) + step_train = TrainingStep( + name="pytorch-train", + estimator=pytorch_estimator, + inputs=inputs, + ) + + step_register = RegisterModel( + name="pytorch-register-model", + estimator=pytorch_estimator, + model_data=step_train.properties.ModelArtifacts.S3ModelArtifacts, + content_types=["*"], + response_types=["*"], + description="test-description", + sample_payload_url=sample_payload_url, + task=task, + framework=framework, + framework_version=framework_version, + nearest_model_name=nearest_model_name, + data_input_configuration=data_input_configuration, + ) + + model = Model( + image_uri=pytorch_estimator.training_image_uri(), + model_data=step_train.properties.ModelArtifacts.S3ModelArtifacts, + sagemaker_session=sagemaker_session, + role=role, + ) + model_inputs = CreateModelInput( + instance_type="ml.m5.large", + accelerator_type="ml.eia1.medium", + ) + step_model = CreateModelStep( + name="pytorch-model", + model=model, + inputs=model_inputs, + ) + + step_cond = ConditionStep( + name="cond-good-enough", + conditions=[ + ConditionGreaterThanOrEqualTo(left=good_enough_input, right=1), + ConditionIn(value=in_condition_input, in_values=["foo", "bar"]), + ], + if_steps=[step_register], + else_steps=[step_model], + depends_on=[step_train], + ) + + pipeline = Pipeline( + name=pipeline_name, + parameters=[ + in_condition_input, + good_enough_input, + instance_count, + ], + steps=[step_train, step_cond], + sagemaker_session=sagemaker_session, + ) + + try: + response = pipeline.create(role) + create_arn = response["PipelineArn"] + assert re.match( + rf"arn:aws:sagemaker:{region_name}:\d{{12}}:pipeline/{pipeline_name}", + create_arn, + ) + + execution = pipeline.start(parameters={}) + assert re.match( + rf"arn:aws:sagemaker:{region_name}:\d{{12}}:pipeline/{pipeline_name}/execution/", + execution.arn, + ) + + execution = pipeline.start(parameters={"GoodEnoughInput": 0}) + assert re.match( + rf"arn:aws:sagemaker:{region_name}:\d{{12}}:pipeline/{pipeline_name}/execution/", + execution.arn, + ) + finally: + try: + pipeline.delete() + except Exception: + pass + + +def test_conditional_pytorch_training_model_registration_with_one_instance_types( + sagemaker_session, + role, + cpu_instance_type, + pipeline_name, + region_name, +): + base_dir = os.path.join(DATA_DIR, "pytorch_mnist") + entry_point = os.path.join(base_dir, "mnist.py") + input_path = sagemaker_session.upload_data( + path=os.path.join(base_dir, "training"), + key_prefix="integ-test-data/pytorch_mnist/training", + ) + inputs = TrainingInput(s3_data=input_path) + + instance_count = ParameterInteger(name="InstanceCount", default_value=1) + instance_type = "ml.m5.xlarge" + good_enough_input = ParameterInteger(name="GoodEnoughInput", default_value=1) + in_condition_input = ParameterString(name="Foo", default_value="Foo") + + task = "IMAGE_CLASSIFICATION" + sample_payload_url = "s3://test-bucket/model" + framework = "TENSORFLOW" + framework_version = "2.9" + nearest_model_name = "resnet50" + data_input_configuration = '{"input_1":[1,224,224,3]}' + + # If image_uri is not provided, the instance_type should not be a pipeline variable + # since instance_type is used to retrieve image_uri in compile time (PySDK) + pytorch_estimator = PyTorch( + entry_point=entry_point, + role=role, + framework_version="1.5.0", + py_version="py3", + instance_count=instance_count, + instance_type=instance_type, + sagemaker_session=sagemaker_session, + ) + step_train = TrainingStep( + name="pytorch-train", + estimator=pytorch_estimator, + inputs=inputs, + ) + + step_register = RegisterModel( + name="pytorch-register-model", + estimator=pytorch_estimator, + model_data=step_train.properties.ModelArtifacts.S3ModelArtifacts, + content_types=["*"], + response_types=["*"], + inference_instances=["*"], + description="test-description", + sample_payload_url=sample_payload_url, + task=task, + framework=framework, + framework_version=framework_version, + nearest_model_name=nearest_model_name, + data_input_configuration=data_input_configuration, + ) + + model = Model( + image_uri=pytorch_estimator.training_image_uri(), + model_data=step_train.properties.ModelArtifacts.S3ModelArtifacts, + sagemaker_session=sagemaker_session, + role=role, + ) + model_inputs = CreateModelInput( + instance_type="ml.m5.large", + accelerator_type="ml.eia1.medium", + ) + step_model = CreateModelStep( + name="pytorch-model", + model=model, + inputs=model_inputs, + ) + + step_cond = ConditionStep( + name="cond-good-enough", + conditions=[ + ConditionGreaterThanOrEqualTo(left=good_enough_input, right=1), + ConditionIn(value=in_condition_input, in_values=["foo", "bar"]), + ], + if_steps=[step_register], + else_steps=[step_model], + depends_on=[step_train], + ) + + pipeline = Pipeline( + name=pipeline_name, + parameters=[ + in_condition_input, + good_enough_input, + instance_count, + ], + steps=[step_train, step_cond], + sagemaker_session=sagemaker_session, + ) + + try: + response = pipeline.create(role) + create_arn = response["PipelineArn"] + assert re.match( + rf"arn:aws:sagemaker:{region_name}:\d{{12}}:pipeline/{pipeline_name}", + create_arn, + ) + + execution = pipeline.start(parameters={}) + assert re.match( + rf"arn:aws:sagemaker:{region_name}:\d{{12}}:pipeline/{pipeline_name}/execution/", + execution.arn, + ) + + execution = pipeline.start(parameters={"GoodEnoughInput": 0}) + assert re.match( + rf"arn:aws:sagemaker:{region_name}:\d{{12}}:pipeline/{pipeline_name}/execution/", + execution.arn, + ) + finally: + try: + pipeline.delete() + except Exception: + pass + + def test_mxnet_model_registration( sagemaker_session, role, diff --git a/tests/unit/sagemaker/workflow/test_pipeline_session.py b/tests/unit/sagemaker/workflow/test_pipeline_session.py index 96e4032a74..c36f1ff117 100644 --- a/tests/unit/sagemaker/workflow/test_pipeline_session.py +++ b/tests/unit/sagemaker/workflow/test_pipeline_session.py @@ -228,8 +228,68 @@ def test_pipeline_session_context_for_model_step_without_instance_types( ], "SupportedContentTypes": ["text/csv"], "SupportedResponseMIMETypes": ["text/csv"], - "SupportedRealtimeInferenceInstanceTypes": None, - "SupportedTransformInstanceTypes": None, + }, + "CertifyForMarketplace": False, + "ModelApprovalStatus": "PendingManualApproval", + "SamplePayloadUrl": "s3://test-bucket/model", + "Task": "IMAGE_CLASSIFICATION", + } + + assert register_step_args.create_model_package_request == expected_output + + +def test_pipeline_session_context_for_model_step_with_one_instance_types( + pipeline_session_mock, +): + model = Model( + name="MyModel", + image_uri="fakeimage", + model_data=ParameterString(name="ModelData", default_value="s3://my-bucket/file"), + sagemaker_session=pipeline_session_mock, + entry_point=f"{DATA_DIR}/dummy_script.py", + source_dir=f"{DATA_DIR}", + role=_ROLE, + ) + register_step_args = model.register( + content_types=["text/csv"], + response_types=["text/csv"], + inference_instances=["ml.t2.medium", "ml.m5.xlarge"], + model_package_group_name="MyModelPackageGroup", + task="IMAGE_CLASSIFICATION", + sample_payload_url="s3://test-bucket/model", + framework="TENSORFLOW", + framework_version="2.9", + nearest_model_name="resnet50", + data_input_configuration='{"input_1":[1,224,224,3]}', + ) + + expected_output = { + "ModelPackageGroupName": "MyModelPackageGroup", + "InferenceSpecification": { + "Containers": [ + { + "Image": "fakeimage", + "Environment": { + "SAGEMAKER_PROGRAM": "dummy_script.py", + "SAGEMAKER_SUBMIT_DIRECTORY": "/opt/ml/model/code", + "SAGEMAKER_CONTAINER_LOG_LEVEL": "20", + "SAGEMAKER_REGION": "us-west-2", + }, + "ModelDataUrl": ParameterString( + name="ModelData", + default_value="s3://my-bucket/file", + ), + "Framework": "TENSORFLOW", + "FrameworkVersion": "2.9", + "NearestModelName": "resnet50", + "ModelInput": { + "DataInputConfig": '{"input_1":[1,224,224,3]}', + }, + } + ], + "SupportedContentTypes": ["text/csv"], + "SupportedResponseMIMETypes": ["text/csv"], + "SupportedRealtimeInferenceInstanceTypes": ["ml.t2.medium", "ml.m5.xlarge"], }, "CertifyForMarketplace": False, "ModelApprovalStatus": "PendingManualApproval", diff --git a/tests/unit/test_session.py b/tests/unit/test_session.py index 1fd58ea531..7bb40ba91d 100644 --- a/tests/unit/test_session.py +++ b/tests/unit/test_session.py @@ -2485,8 +2485,70 @@ def test_create_model_package_from_containers_without_instance_types(sagemaker_s "Containers": containers, "SupportedContentTypes": content_types, "SupportedResponseMIMETypes": response_types, - "SupportedRealtimeInferenceInstanceTypes": None, - "SupportedTransformInstanceTypes": None, + }, + "ModelPackageDescription": description, + "ModelMetrics": model_metrics, + "MetadataProperties": metadata_properties, + "CertifyForMarketplace": marketplace_cert, + "ModelApprovalStatus": approval_status, + "DriftCheckBaselines": drift_check_baselines, + "CustomerMetadataProperties": customer_metadata_properties, + } + sagemaker_session.sagemaker_client.create_model_package.assert_called_with(**expected_args) + + +def test_create_model_package_from_containers_with_one_instance_types(sagemaker_session): + model_package_name = "sagemaker-model-package" + containers = ["dummy-container"] + content_types = ["application/json"] + response_types = ["application/json"] + transform_instances = ["ml.m5.xlarge"] + model_metrics = { + "Bias": { + "ContentType": "content-type", + "S3Uri": "s3://...", + } + } + drift_check_baselines = { + "Bias": { + "ConfigFile": { + "ContentType": "content-type", + "S3Uri": "s3://...", + } + } + } + + metadata_properties = { + "CommitId": "test-commit-id", + "Repository": "test-repository", + "GeneratedBy": "sagemaker-python-sdk", + "ProjectId": "unit-test", + } + marketplace_cert = (True,) + approval_status = ("Approved",) + description = "description" + customer_metadata_properties = {"key1": "value1"} + sagemaker_session.create_model_package_from_containers( + containers=containers, + content_types=content_types, + response_types=response_types, + transform_instances=transform_instances, + model_package_name=model_package_name, + model_metrics=model_metrics, + metadata_properties=metadata_properties, + marketplace_cert=marketplace_cert, + approval_status=approval_status, + description=description, + drift_check_baselines=drift_check_baselines, + customer_metadata_properties=customer_metadata_properties, + ) + expected_args = { + "ModelPackageName": model_package_name, + "InferenceSpecification": { + "Containers": containers, + "SupportedContentTypes": content_types, + "SupportedResponseMIMETypes": response_types, + "SupportedTransformInstanceTypes": transform_instances, }, "ModelPackageDescription": description, "ModelMetrics": model_metrics, From ff7e2bf2946fcbf4bb7e77d59633e98691c31366 Mon Sep 17 00:00:00 2001 From: Rahul Venkatesh Date: Tue, 12 Jul 2022 15:46:26 +0530 Subject: [PATCH 2/4] remove failing integ test --- .../test_model_create_and_registration.py | 247 ------------------ 1 file changed, 247 deletions(-) diff --git a/tests/integ/sagemaker/workflow/test_model_create_and_registration.py b/tests/integ/sagemaker/workflow/test_model_create_and_registration.py index 63aae962fa..d0f617a266 100644 --- a/tests/integ/sagemaker/workflow/test_model_create_and_registration.py +++ b/tests/integ/sagemaker/workflow/test_model_create_and_registration.py @@ -199,253 +199,6 @@ def test_conditional_pytorch_training_model_registration( pass -def test_conditional_pytorch_training_model_registration_without_instance_types( - sagemaker_session, - role, - cpu_instance_type, - pipeline_name, - region_name, -): - base_dir = os.path.join(DATA_DIR, "pytorch_mnist") - entry_point = os.path.join(base_dir, "mnist.py") - input_path = sagemaker_session.upload_data( - path=os.path.join(base_dir, "training"), - key_prefix="integ-test-data/pytorch_mnist/training", - ) - inputs = TrainingInput(s3_data=input_path) - - instance_count = ParameterInteger(name="InstanceCount", default_value=1) - instance_type = "ml.m5.xlarge" - good_enough_input = ParameterInteger(name="GoodEnoughInput", default_value=1) - in_condition_input = ParameterString(name="Foo", default_value="Foo") - - task = "IMAGE_CLASSIFICATION" - sample_payload_url = "s3://test-bucket/model" - framework = "TENSORFLOW" - framework_version = "2.9" - nearest_model_name = "resnet50" - data_input_configuration = '{"input_1":[1,224,224,3]}' - - # If image_uri is not provided, the instance_type should not be a pipeline variable - # since instance_type is used to retrieve image_uri in compile time (PySDK) - pytorch_estimator = PyTorch( - entry_point=entry_point, - role=role, - framework_version="1.5.0", - py_version="py3", - instance_count=instance_count, - instance_type=instance_type, - sagemaker_session=sagemaker_session, - ) - step_train = TrainingStep( - name="pytorch-train", - estimator=pytorch_estimator, - inputs=inputs, - ) - - step_register = RegisterModel( - name="pytorch-register-model", - estimator=pytorch_estimator, - model_data=step_train.properties.ModelArtifacts.S3ModelArtifacts, - content_types=["*"], - response_types=["*"], - description="test-description", - sample_payload_url=sample_payload_url, - task=task, - framework=framework, - framework_version=framework_version, - nearest_model_name=nearest_model_name, - data_input_configuration=data_input_configuration, - ) - - model = Model( - image_uri=pytorch_estimator.training_image_uri(), - model_data=step_train.properties.ModelArtifacts.S3ModelArtifacts, - sagemaker_session=sagemaker_session, - role=role, - ) - model_inputs = CreateModelInput( - instance_type="ml.m5.large", - accelerator_type="ml.eia1.medium", - ) - step_model = CreateModelStep( - name="pytorch-model", - model=model, - inputs=model_inputs, - ) - - step_cond = ConditionStep( - name="cond-good-enough", - conditions=[ - ConditionGreaterThanOrEqualTo(left=good_enough_input, right=1), - ConditionIn(value=in_condition_input, in_values=["foo", "bar"]), - ], - if_steps=[step_register], - else_steps=[step_model], - depends_on=[step_train], - ) - - pipeline = Pipeline( - name=pipeline_name, - parameters=[ - in_condition_input, - good_enough_input, - instance_count, - ], - steps=[step_train, step_cond], - sagemaker_session=sagemaker_session, - ) - - try: - response = pipeline.create(role) - create_arn = response["PipelineArn"] - assert re.match( - rf"arn:aws:sagemaker:{region_name}:\d{{12}}:pipeline/{pipeline_name}", - create_arn, - ) - - execution = pipeline.start(parameters={}) - assert re.match( - rf"arn:aws:sagemaker:{region_name}:\d{{12}}:pipeline/{pipeline_name}/execution/", - execution.arn, - ) - - execution = pipeline.start(parameters={"GoodEnoughInput": 0}) - assert re.match( - rf"arn:aws:sagemaker:{region_name}:\d{{12}}:pipeline/{pipeline_name}/execution/", - execution.arn, - ) - finally: - try: - pipeline.delete() - except Exception: - pass - - -def test_conditional_pytorch_training_model_registration_with_one_instance_types( - sagemaker_session, - role, - cpu_instance_type, - pipeline_name, - region_name, -): - base_dir = os.path.join(DATA_DIR, "pytorch_mnist") - entry_point = os.path.join(base_dir, "mnist.py") - input_path = sagemaker_session.upload_data( - path=os.path.join(base_dir, "training"), - key_prefix="integ-test-data/pytorch_mnist/training", - ) - inputs = TrainingInput(s3_data=input_path) - - instance_count = ParameterInteger(name="InstanceCount", default_value=1) - instance_type = "ml.m5.xlarge" - good_enough_input = ParameterInteger(name="GoodEnoughInput", default_value=1) - in_condition_input = ParameterString(name="Foo", default_value="Foo") - - task = "IMAGE_CLASSIFICATION" - sample_payload_url = "s3://test-bucket/model" - framework = "TENSORFLOW" - framework_version = "2.9" - nearest_model_name = "resnet50" - data_input_configuration = '{"input_1":[1,224,224,3]}' - - # If image_uri is not provided, the instance_type should not be a pipeline variable - # since instance_type is used to retrieve image_uri in compile time (PySDK) - pytorch_estimator = PyTorch( - entry_point=entry_point, - role=role, - framework_version="1.5.0", - py_version="py3", - instance_count=instance_count, - instance_type=instance_type, - sagemaker_session=sagemaker_session, - ) - step_train = TrainingStep( - name="pytorch-train", - estimator=pytorch_estimator, - inputs=inputs, - ) - - step_register = RegisterModel( - name="pytorch-register-model", - estimator=pytorch_estimator, - model_data=step_train.properties.ModelArtifacts.S3ModelArtifacts, - content_types=["*"], - response_types=["*"], - inference_instances=["*"], - description="test-description", - sample_payload_url=sample_payload_url, - task=task, - framework=framework, - framework_version=framework_version, - nearest_model_name=nearest_model_name, - data_input_configuration=data_input_configuration, - ) - - model = Model( - image_uri=pytorch_estimator.training_image_uri(), - model_data=step_train.properties.ModelArtifacts.S3ModelArtifacts, - sagemaker_session=sagemaker_session, - role=role, - ) - model_inputs = CreateModelInput( - instance_type="ml.m5.large", - accelerator_type="ml.eia1.medium", - ) - step_model = CreateModelStep( - name="pytorch-model", - model=model, - inputs=model_inputs, - ) - - step_cond = ConditionStep( - name="cond-good-enough", - conditions=[ - ConditionGreaterThanOrEqualTo(left=good_enough_input, right=1), - ConditionIn(value=in_condition_input, in_values=["foo", "bar"]), - ], - if_steps=[step_register], - else_steps=[step_model], - depends_on=[step_train], - ) - - pipeline = Pipeline( - name=pipeline_name, - parameters=[ - in_condition_input, - good_enough_input, - instance_count, - ], - steps=[step_train, step_cond], - sagemaker_session=sagemaker_session, - ) - - try: - response = pipeline.create(role) - create_arn = response["PipelineArn"] - assert re.match( - rf"arn:aws:sagemaker:{region_name}:\d{{12}}:pipeline/{pipeline_name}", - create_arn, - ) - - execution = pipeline.start(parameters={}) - assert re.match( - rf"arn:aws:sagemaker:{region_name}:\d{{12}}:pipeline/{pipeline_name}/execution/", - execution.arn, - ) - - execution = pipeline.start(parameters={"GoodEnoughInput": 0}) - assert re.match( - rf"arn:aws:sagemaker:{region_name}:\d{{12}}:pipeline/{pipeline_name}/execution/", - execution.arn, - ) - finally: - try: - pipeline.delete() - except Exception: - pass - - def test_mxnet_model_registration( sagemaker_session, role, From bcb3425ae962049cbed8d6d5f1adc495efd3920d Mon Sep 17 00:00:00 2001 From: Rahul Venkatesh Date: Tue, 12 Jul 2022 19:46:16 +0530 Subject: [PATCH 3/4] fix: make instance_type optional for model registry model package and mandatory for marketplace model package --- src/sagemaker/session.py | 34 ++++++++++++------- .../workflow/test_pipeline_session.py | 30 ++++++++++++++++ tests/unit/test_session.py | 31 +++++++++++++---- 3 files changed, 76 insertions(+), 19 deletions(-) diff --git a/src/sagemaker/session.py b/src/sagemaker/session.py index e5e9b171e9..fac629562d 100644 --- a/src/sagemaker/session.py +++ b/src/sagemaker/session.py @@ -4500,18 +4500,28 @@ def get_create_model_package_request( "SupportedContentTypes": content_types, "SupportedResponseMIMETypes": response_types, } - if inference_instances is not None: - inference_specification.update( - { - "SupportedRealtimeInferenceInstanceTypes": inference_instances, - } - ) - if transform_instances is not None: - inference_specification.update( - { - "SupportedTransformInstanceTypes": transform_instances, - } - ) + if model_package_group_name is not None: + if inference_instances is not None: + inference_specification.update( + { + "SupportedRealtimeInferenceInstanceTypes": inference_instances, + } + ) + if transform_instances is not None: + inference_specification.update( + { + "SupportedTransformInstanceTypes": transform_instances, + } + ) + else: + if not all([inference_instances, transform_instances]): + raise ValueError( + "inference_instances and transform_instances " "must be provided if model_package_group_name is not present." + ) + inference_specification.update({ + "SupportedRealtimeInferenceInstanceTypes": inference_instances, + "SupportedTransformInstanceTypes": transform_instances, + }) request_dict["InferenceSpecification"] = inference_specification request_dict["CertifyForMarketplace"] = marketplace_cert request_dict["ModelApprovalStatus"] = approval_status diff --git a/tests/unit/sagemaker/workflow/test_pipeline_session.py b/tests/unit/sagemaker/workflow/test_pipeline_session.py index c36f1ff117..32b0b57f73 100644 --- a/tests/unit/sagemaker/workflow/test_pipeline_session.py +++ b/tests/unit/sagemaker/workflow/test_pipeline_session.py @@ -298,3 +298,33 @@ def test_pipeline_session_context_for_model_step_with_one_instance_types( } assert register_step_args.create_model_package_request == expected_output + +def test_pipeline_session_context_for_model_step_without_model_package_group_name( + pipeline_session_mock, +): + model = Model( + name="MyModel", + image_uri="fakeimage", + model_data=ParameterString(name="ModelData", default_value="s3://my-bucket/file"), + sagemaker_session=pipeline_session_mock, + entry_point=f"{DATA_DIR}/dummy_script.py", + source_dir=f"{DATA_DIR}", + role=_ROLE, + ) + with pytest.raises(ValueError) as error: + model.register( + content_types=["text/csv"], + response_types=["text/csv"], + inference_instances=["ml.t2.medium", "ml.m5.xlarge"], + model_package_name="MyModelPackageName", + task="IMAGE_CLASSIFICATION", + sample_payload_url="s3://test-bucket/model", + framework="TENSORFLOW", + framework_version="2.9", + nearest_model_name="resnet50", + data_input_configuration='{"input_1":[1,224,224,3]}', + ) + assert ( + "inference_inferences and transform_instances " + "must be provided if model_package_group_name is not present." == str(error) + ) diff --git a/tests/unit/test_session.py b/tests/unit/test_session.py index 7bb40ba91d..bbf92dde42 100644 --- a/tests/unit/test_session.py +++ b/tests/unit/test_session.py @@ -2355,10 +2355,27 @@ def test_create_model_package_from_containers_incomplete_args(sagemaker_session) containers=containers, ) assert ( - "content_types, response_types, inference_inferences and transform_instances " + "content_types and response_types " "must be provided if containers is present." == str(error) ) +def test_create_model_package_from_containers_without_model_package_group_name(sagemaker_session): + model_package_name = "sagemaker-model-package" + containers = ["dummy-container"] + content_types = ["application/json"] + response_types = ["application/json"] + with pytest.raises(ValueError) as error: + sagemaker_session.create_model_package_from_containers( + model_package_name=model_package_name, + containers=containers, + content_types=content_types, + response_types=response_types, + ) + assert ( + "inference_inferences and transform_instances " + "must be provided if model_package_group_name is not present." == str(error) + ) + def test_create_model_package_from_containers_all_args(sagemaker_session): model_package_name = "sagemaker-model-package" @@ -2437,7 +2454,7 @@ def test_create_model_package_from_containers_all_args(sagemaker_session): def test_create_model_package_from_containers_without_instance_types(sagemaker_session): - model_package_name = "sagemaker-model-package" + model_package_group_name = "sagemaker-model-package-group-name-1.0" containers = ["dummy-container"] content_types = ["application/json"] response_types = ["application/json"] @@ -2470,7 +2487,7 @@ def test_create_model_package_from_containers_without_instance_types(sagemaker_s containers=containers, content_types=content_types, response_types=response_types, - model_package_name=model_package_name, + model_package_group_name=model_package_group_name, model_metrics=model_metrics, metadata_properties=metadata_properties, marketplace_cert=marketplace_cert, @@ -2480,7 +2497,7 @@ def test_create_model_package_from_containers_without_instance_types(sagemaker_s customer_metadata_properties=customer_metadata_properties, ) expected_args = { - "ModelPackageName": model_package_name, + "ModelPackageGroupName": model_package_group_name, "InferenceSpecification": { "Containers": containers, "SupportedContentTypes": content_types, @@ -2498,7 +2515,7 @@ def test_create_model_package_from_containers_without_instance_types(sagemaker_s def test_create_model_package_from_containers_with_one_instance_types(sagemaker_session): - model_package_name = "sagemaker-model-package" + model_package_group_name = "sagemaker-model-package-group-name-1.0" containers = ["dummy-container"] content_types = ["application/json"] response_types = ["application/json"] @@ -2533,7 +2550,7 @@ def test_create_model_package_from_containers_with_one_instance_types(sagemaker_ content_types=content_types, response_types=response_types, transform_instances=transform_instances, - model_package_name=model_package_name, + model_package_group_name=model_package_group_name, model_metrics=model_metrics, metadata_properties=metadata_properties, marketplace_cert=marketplace_cert, @@ -2543,7 +2560,7 @@ def test_create_model_package_from_containers_with_one_instance_types(sagemaker_ customer_metadata_properties=customer_metadata_properties, ) expected_args = { - "ModelPackageName": model_package_name, + "ModelPackageGroupName": model_package_group_name, "InferenceSpecification": { "Containers": containers, "SupportedContentTypes": content_types, From 95af7581be63b15ada1bcf8a046300b12ce0d0f7 Mon Sep 17 00:00:00 2001 From: Rahul Venkatesh Date: Tue, 12 Jul 2022 20:36:57 +0530 Subject: [PATCH 4/4] fix: black-check and flake8 errors --- src/sagemaker/session.py | 13 ++++++++----- .../sagemaker/workflow/test_pipeline_session.py | 1 + tests/unit/test_session.py | 1 + 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/sagemaker/session.py b/src/sagemaker/session.py index fac629562d..145bf41cbe 100644 --- a/src/sagemaker/session.py +++ b/src/sagemaker/session.py @@ -4516,12 +4516,15 @@ def get_create_model_package_request( else: if not all([inference_instances, transform_instances]): raise ValueError( - "inference_instances and transform_instances " "must be provided if model_package_group_name is not present." + "inference_instances and transform_instances " + "must be provided if model_package_group_name is not present." ) - inference_specification.update({ - "SupportedRealtimeInferenceInstanceTypes": inference_instances, - "SupportedTransformInstanceTypes": transform_instances, - }) + inference_specification.update( + { + "SupportedRealtimeInferenceInstanceTypes": inference_instances, + "SupportedTransformInstanceTypes": transform_instances, + } + ) request_dict["InferenceSpecification"] = inference_specification request_dict["CertifyForMarketplace"] = marketplace_cert request_dict["ModelApprovalStatus"] = approval_status diff --git a/tests/unit/sagemaker/workflow/test_pipeline_session.py b/tests/unit/sagemaker/workflow/test_pipeline_session.py index 32b0b57f73..91c7bbb675 100644 --- a/tests/unit/sagemaker/workflow/test_pipeline_session.py +++ b/tests/unit/sagemaker/workflow/test_pipeline_session.py @@ -299,6 +299,7 @@ def test_pipeline_session_context_for_model_step_with_one_instance_types( assert register_step_args.create_model_package_request == expected_output + def test_pipeline_session_context_for_model_step_without_model_package_group_name( pipeline_session_mock, ): diff --git a/tests/unit/test_session.py b/tests/unit/test_session.py index bbf92dde42..78df274b71 100644 --- a/tests/unit/test_session.py +++ b/tests/unit/test_session.py @@ -2359,6 +2359,7 @@ def test_create_model_package_from_containers_incomplete_args(sagemaker_session) "must be provided if containers is present." == str(error) ) + def test_create_model_package_from_containers_without_model_package_group_name(sagemaker_session): model_package_name = "sagemaker-model-package" containers = ["dummy-container"]