From 1dbba050ea07cc1084ed9db5c332af7ce47e07ff Mon Sep 17 00:00:00 2001 From: Samrudhi Sharma Date: Wed, 15 May 2024 08:07:49 -0700 Subject: [PATCH 1/5] Fix: Image URI should take precedence for HF models --- .../serve/builder/transformers_builder.py | 10 +++-- .../builder/test_transformers_builder.py | 45 ++++++++++++++++++- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/sagemaker/serve/builder/transformers_builder.py b/src/sagemaker/serve/builder/transformers_builder.py index 3d84e314df..ead9b7425f 100644 --- a/src/sagemaker/serve/builder/transformers_builder.py +++ b/src/sagemaker/serve/builder/transformers_builder.py @@ -132,17 +132,20 @@ def _create_transformers_model(self) -> Type[Model]: vpc_config=self.vpc_config, ) - if self.mode == Mode.LOCAL_CONTAINER: + if not self.image_uri and self.mode == Mode.LOCAL_CONTAINER: self.image_uri = pysdk_model.serving_image_uri( self.sagemaker_session.boto_region_name, "local" ) - else: + elif not self.image_uri: self.image_uri = pysdk_model.serving_image_uri( self.sagemaker_session.boto_region_name, self.instance_type ) logger.info("Detected %s. Proceeding with the the deployment.", self.image_uri) + if not pysdk_model.image_uri: + pysdk_model.image_uri = self.image_uri + self._original_deploy = pysdk_model.deploy pysdk_model.deploy = self._transformers_model_builder_deploy_wrapper return pysdk_model @@ -251,13 +254,14 @@ def _set_instance(self, **kwargs): if self.mode == Mode.SAGEMAKER_ENDPOINT: if self.nb_instance_type and "instance_type" not in kwargs: kwargs.update({"instance_type": self.nb_instance_type}) + logger.info("Setting instance type to %s", self.nb_instance_type) elif self.instance_type and "instance_type" not in kwargs: kwargs.update({"instance_type": self.instance_type}) + logger.info("Setting instance type to %s", self.instance_type) else: raise ValueError( "Instance type must be provided when deploying to SageMaker Endpoint mode." ) - logger.info("Setting instance type to %s", self.instance_type) def _get_supported_version(self, hf_config, hugging_face_version, base_fw): """Uses the hugging face json config to pick supported versions""" diff --git a/tests/unit/sagemaker/serve/builder/test_transformers_builder.py b/tests/unit/sagemaker/serve/builder/test_transformers_builder.py index e17364f22d..93297e0fc4 100644 --- a/tests/unit/sagemaker/serve/builder/test_transformers_builder.py +++ b/tests/unit/sagemaker/serve/builder/test_transformers_builder.py @@ -16,7 +16,7 @@ import unittest from sagemaker.serve.builder.model_builder import ModelBuilder from sagemaker.serve.mode.function_pointers import Mode -from tests.unit.sagemaker.serve.constants import MOCK_VPC_CONFIG +from tests.unit.sagemaker.serve.constants import MOCK_IMAGE_CONFIG, MOCK_VPC_CONFIG from sagemaker.serve.utils.predictors import TransformersLocalModePredictor @@ -100,3 +100,46 @@ def test_build_deploy_for_transformers_local_container_and_remote_container( with self.assertRaises(ValueError) as _: model.deploy(mode=Mode.IN_PROCESS) + + + @patch( + "sagemaker.serve.builder.transformers_builder._get_nb_instance", + return_value="ml.g5.24xlarge", + ) + @patch("sagemaker.serve.builder.transformers_builder._capture_telemetry", side_effect=None) + def test_image_uri( + self, + mock_get_nb_instance, + mock_telemetry, + ): + builder = ModelBuilder( + model=mock_model_id, + schema_builder=mock_schema_builder, + mode=Mode.LOCAL_CONTAINER, + vpc_config=MOCK_VPC_CONFIG, + image_config=MOCK_IMAGE_CONFIG, + ) + + builder._prepare_for_mode = MagicMock() + builder._prepare_for_mode.side_effect = None + + model = builder.build() + builder.serve_settings.telemetry_opt_out = True + + builder.modes[str(Mode.LOCAL_CONTAINER)] = MagicMock() + predictor = model.deploy(model_data_download_timeout=1800) + + assert model.image_config == MOCK_IMAGE_CONFIG + assert model.vpc_config == MOCK_VPC_CONFIG + assert builder.env_vars["MODEL_LOADING_TIMEOUT"] == "1800" + assert isinstance(predictor, TransformersLocalModePredictor) + + assert builder.nb_instance_type == "ml.g5.24xlarge" + + builder._original_deploy = MagicMock() + builder._prepare_for_mode.return_value = (None, {}) + predictor = model.deploy(mode=Mode.SAGEMAKER_ENDPOINT, role="mock_role_arn") + assert "HF_MODEL_ID" in model.env + + with self.assertRaises(ValueError) as _: + model.deploy(mode=Mode.IN_PROCESS) From 3686dd98893dc6a08d28908e8a5d400a5db71308 Mon Sep 17 00:00:00 2001 From: Samrudhi Sharma Date: Wed, 15 May 2024 15:20:13 +0000 Subject: [PATCH 2/5] Fix formatting --- .../sagemaker/serve/builder/test_transformers_builder.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/unit/sagemaker/serve/builder/test_transformers_builder.py b/tests/unit/sagemaker/serve/builder/test_transformers_builder.py index 93297e0fc4..e4630017ed 100644 --- a/tests/unit/sagemaker/serve/builder/test_transformers_builder.py +++ b/tests/unit/sagemaker/serve/builder/test_transformers_builder.py @@ -101,16 +101,15 @@ def test_build_deploy_for_transformers_local_container_and_remote_container( with self.assertRaises(ValueError) as _: model.deploy(mode=Mode.IN_PROCESS) - @patch( "sagemaker.serve.builder.transformers_builder._get_nb_instance", return_value="ml.g5.24xlarge", ) @patch("sagemaker.serve.builder.transformers_builder._capture_telemetry", side_effect=None) def test_image_uri( - self, - mock_get_nb_instance, - mock_telemetry, + self, + mock_get_nb_instance, + mock_telemetry, ): builder = ModelBuilder( model=mock_model_id, From 8cad99f2b8f46101ac196ac0ca1701cb2185e7b5 Mon Sep 17 00:00:00 2001 From: Samrudhi Sharma Date: Wed, 15 May 2024 11:59:35 -0700 Subject: [PATCH 3/5] Fix formatting --- .../serve/builder/test_transformers_builder.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/tests/unit/sagemaker/serve/builder/test_transformers_builder.py b/tests/unit/sagemaker/serve/builder/test_transformers_builder.py index e4630017ed..b37fc127a5 100644 --- a/tests/unit/sagemaker/serve/builder/test_transformers_builder.py +++ b/tests/unit/sagemaker/serve/builder/test_transformers_builder.py @@ -16,7 +16,7 @@ import unittest from sagemaker.serve.builder.model_builder import ModelBuilder from sagemaker.serve.mode.function_pointers import Mode -from tests.unit.sagemaker.serve.constants import MOCK_IMAGE_CONFIG, MOCK_VPC_CONFIG +from tests.unit.sagemaker.serve.constants import MOCK_VPC_CONFIG from sagemaker.serve.utils.predictors import TransformersLocalModePredictor @@ -58,6 +58,9 @@ mock_schema_builder = MagicMock() mock_schema_builder.sample_input = mock_sample_input mock_schema_builder.sample_output = mock_sample_output +MOCK_IMAGE_CONFIG = \ + "763104351884.dkr.ecr.us-west-2.amazonaws.com/" \ + "huggingface-pytorch-inference:2.0.0-transformers4.28.1-gpu-py310-cu118-ubuntu20.04-v1.0" class TestTransformersBuilder(unittest.TestCase): @@ -115,8 +118,7 @@ def test_image_uri( model=mock_model_id, schema_builder=mock_schema_builder, mode=Mode.LOCAL_CONTAINER, - vpc_config=MOCK_VPC_CONFIG, - image_config=MOCK_IMAGE_CONFIG, + image_uri=MOCK_IMAGE_CONFIG, ) builder._prepare_for_mode = MagicMock() @@ -128,17 +130,11 @@ def test_image_uri( builder.modes[str(Mode.LOCAL_CONTAINER)] = MagicMock() predictor = model.deploy(model_data_download_timeout=1800) - assert model.image_config == MOCK_IMAGE_CONFIG - assert model.vpc_config == MOCK_VPC_CONFIG + assert builder.image_uri == MOCK_IMAGE_CONFIG assert builder.env_vars["MODEL_LOADING_TIMEOUT"] == "1800" assert isinstance(predictor, TransformersLocalModePredictor) assert builder.nb_instance_type == "ml.g5.24xlarge" - builder._original_deploy = MagicMock() - builder._prepare_for_mode.return_value = (None, {}) - predictor = model.deploy(mode=Mode.SAGEMAKER_ENDPOINT, role="mock_role_arn") - assert "HF_MODEL_ID" in model.env - with self.assertRaises(ValueError) as _: model.deploy(mode=Mode.IN_PROCESS) From 599c9a607f0d7423eff9bb26eefd56ca78b53e0c Mon Sep 17 00:00:00 2001 From: Samrudhi Sharma Date: Wed, 15 May 2024 19:00:35 +0000 Subject: [PATCH 4/5] Fix formatting --- .../sagemaker/serve/builder/test_transformers_builder.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/unit/sagemaker/serve/builder/test_transformers_builder.py b/tests/unit/sagemaker/serve/builder/test_transformers_builder.py index b37fc127a5..077e906522 100644 --- a/tests/unit/sagemaker/serve/builder/test_transformers_builder.py +++ b/tests/unit/sagemaker/serve/builder/test_transformers_builder.py @@ -58,9 +58,10 @@ mock_schema_builder = MagicMock() mock_schema_builder.sample_input = mock_sample_input mock_schema_builder.sample_output = mock_sample_output -MOCK_IMAGE_CONFIG = \ - "763104351884.dkr.ecr.us-west-2.amazonaws.com/" \ +MOCK_IMAGE_CONFIG = ( + "763104351884.dkr.ecr.us-west-2.amazonaws.com/" "huggingface-pytorch-inference:2.0.0-transformers4.28.1-gpu-py310-cu118-ubuntu20.04-v1.0" +) class TestTransformersBuilder(unittest.TestCase): From baff568c4b7aa67300950b53d237cee675c0dbfd Mon Sep 17 00:00:00 2001 From: Samrudhi Sharma Date: Wed, 15 May 2024 12:28:54 -0700 Subject: [PATCH 5/5] Increase coverage - UT pass --- .../sagemaker/serve/builder/test_transformers_builder.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/unit/sagemaker/serve/builder/test_transformers_builder.py b/tests/unit/sagemaker/serve/builder/test_transformers_builder.py index 077e906522..b7e3db79d6 100644 --- a/tests/unit/sagemaker/serve/builder/test_transformers_builder.py +++ b/tests/unit/sagemaker/serve/builder/test_transformers_builder.py @@ -137,5 +137,10 @@ def test_image_uri( assert builder.nb_instance_type == "ml.g5.24xlarge" + builder._original_deploy = MagicMock() + builder._prepare_for_mode.return_value = (None, {}) + predictor = model.deploy(mode=Mode.SAGEMAKER_ENDPOINT, role="mock_role_arn") + assert "HF_MODEL_ID" in model.env + with self.assertRaises(ValueError) as _: model.deploy(mode=Mode.IN_PROCESS)