Merge commit 'd0edb9f9220e90e44193f02fe88b94325becb9e2' from main.

qiuosier · qiuosier · commit 221d4c7cb2ea · 2023-12-05T14:06:58.000-05:00
diff --git a/ads/common/oci_mixin.py b/ads/common/oci_mixin.py
@@ -33,7 +33,7 @@
 WORK_REQUEST_STOP_STATE = ("SUCCEEDED", "FAILED", "CANCELED")
 DEFAULT_WAIT_TIME = 1200
 DEFAULT_POLL_INTERVAL = 10
-DEFAULT_WORKFLOW_STEPS = 2
+WORK_REQUEST_PERCENTAGE = 100
 
 
 class MergeStrategy(Enum):
@@ -939,7 +939,6 @@ def get_work_request_response(
     def wait_for_progress(
         self,
         work_request_id: str,
-        num_steps: int = DEFAULT_WORKFLOW_STEPS,
         max_wait_time: int = DEFAULT_WAIT_TIME,
         poll_interval: int = DEFAULT_POLL_INTERVAL,
     ):
@@ -949,8 +948,6 @@ def wait_for_progress(
         ----------
         work_request_id: str
             Work Request OCID.
-        num_steps: (int, optional). Defaults to 2.
-            Number of steps for the progress indicator.
         max_wait_time: int
             Maximum amount of time to wait in seconds (Defaults to 1200).
             Negative implies infinite wait time.
@@ -965,13 +962,14 @@ def wait_for_progress(
 
         i = 0
         start_time = time.time()
-        with get_progress_bar(num_steps) as progress:
+        with get_progress_bar(WORK_REQUEST_PERCENTAGE) as progress:
             seconds_since = time.time() - start_time
             exceed_max_time = max_wait_time > 0 and seconds_since >= max_wait_time
             if exceed_max_time:
                 logger.error(f"Max wait time ({max_wait_time} seconds) exceeded.")
+            previous_percent_complete = 0
             while not exceed_max_time and (
-                not work_request_logs or len(work_request_logs) < num_steps
+                not work_request_logs or previous_percent_complete <= WORK_REQUEST_PERCENTAGE
             ):
                 time.sleep(poll_interval)
                 new_work_request_logs = []
@@ -988,9 +986,23 @@ def wait_for_progress(
                     work_request_logs[i:] if work_request_logs else []
                 )
 
-                for wr_item in new_work_request_logs:
-                    progress.update(wr_item.message)
-                    i += 1
+                percent_change = work_request.percent_complete - previous_percent_complete
+                previous_percent_complete = work_request.percent_complete
+
+                if len(new_work_request_logs) > 0:
+                    start_index = True
+                    for wr_item in new_work_request_logs:
+                        if start_index:
+                            progress.update(wr_item.message, percent_change)
+                            start_index = False
+                        else:
+                            progress.update(wr_item.message, 0)
+                        i += 1
+                else:
+                    # if there is new percent change but the new work request logs is empty
+                    # needs to add this percent change to the bar to ensure the final percentage is 100
+                    if percent_change != 0:
+                        progress.update(n=percent_change)
 
                 if work_request and work_request.status in WORK_REQUEST_STOP_STATE:
                     if work_request.status != "SUCCEEDED":
diff --git a/ads/model/generic_model.py b/ads/model/generic_model.py
@@ -67,7 +67,8 @@
     Framework,
     ModelCustomMetadata,
     ModelProvenanceMetadata,
-    ModelTaxonomyMetadata, MetadataCustomCategory,
+    ModelTaxonomyMetadata,
+    MetadataCustomCategory,
 )
 from ads.model.model_metadata_mixin import MetadataMixin
 from ads.model.model_properties import ModelProperties
@@ -1824,18 +1825,19 @@ def _random_display_name(self):
 
     def save(
         self,
-        display_name: Optional[str] = None,
+        bucket_uri: Optional[str] = None,
+        defined_tags: Optional[dict] = None,
         description: Optional[str] = None,
+        display_name: Optional[str] = None,
+        featurestore_dataset=None,
         freeform_tags: Optional[dict] = None,
-        defined_tags: Optional[dict] = None,
         ignore_introspection: Optional[bool] = False,
-        bucket_uri: Optional[str] = None,
+        model_version_set: Optional[Union[str, ModelVersionSet]] = None,
         overwrite_existing_artifact: Optional[bool] = True,
+        parallel_process_count: int = utils.DEFAULT_PARALLEL_PROCESS_COUNT,
         remove_existing_artifact: Optional[bool] = True,
-        model_version_set: Optional[Union[str, ModelVersionSet]] = None,
+        reload: Optional[bool] = True,
         version_label: Optional[str] = None,
-        featurestore_dataset=None,
-        parallel_process_count: int = utils.DEFAULT_PARALLEL_PROCESS_COUNT,
         **kwargs,
     ) -> str:
         """Saves model artifacts to the model catalog.
@@ -1862,7 +1864,7 @@ def save(
         overwrite_existing_artifact: (bool, optional). Defaults to `True`.
             Overwrite target bucket artifact if exists.
         remove_existing_artifact: (bool, optional). Defaults to `True`.
-            Wether artifacts uploaded to object storage bucket need to be removed or not.
+            Whether artifacts uploaded to object storage bucket need to be removed or not.
         model_version_set: (Union[str, ModelVersionSet], optional). Defaults to None.
             The model version set OCID, or model version set name, or `ModelVersionSet` instance.
         version_label: (str, optional). Defaults to None.
@@ -1871,6 +1873,8 @@ def save(
             The feature store dataset
         parallel_process_count: (int, optional)
             The number of worker processes to use in parallel for uploading individual parts of a multipart upload.
+        reload: (bool, optional)
+            Whether to reload to check if `load_model()` works in `score.py`. Default to `True`.
         kwargs:
             project_id: (str, optional).
                 Project OCID. If not specified, the value will be taken either
@@ -1926,12 +1930,17 @@ def save(
                 raise RuntimeInfoInconsistencyError(
                     "`.runtime_info` does not sync with runtime.yaml file. Call "
                     "`.runtime_info.save()` if you updated `runtime_info`. "
-                    "Call `.reload()` if you updated runtime.yaml file."
+                    "Call `.reload_runtime_info()` if you updated runtime.yaml file."
                 )
             # reload to check if load_model works in score.py, i.e.
             # whether the model file has been serialized, and whether it can be loaded
             # successfully.
-            self.reload()
+            if reload:
+                self.reload()
+            else:
+                logger.warning(
+                    "The score.py file has not undergone testing, and this could result in deployment errors. To verify its functionality, please set `reload=True`."
+                )
         except:
             if not self.ignore_conda_error:
                 raise
@@ -1967,11 +1976,15 @@ def save(
         if featurestore_dataset:
             dataset_details = {
                 "dataset-id": featurestore_dataset.id,
-                "dataset-name": featurestore_dataset.name
+                "dataset-name": featurestore_dataset.name,
             }
-            self.metadata_custom.add("featurestore.dataset", value=str(dataset_details),
-                                     category=MetadataCustomCategory.TRAINING_AND_VALIDATION_DATASETS,
-                                     description="feature store dataset", replace=True)
+            self.metadata_custom.add(
+                "featurestore.dataset",
+                value=str(dataset_details),
+                category=MetadataCustomCategory.TRAINING_AND_VALIDATION_DATASETS,
+                description="feature store dataset",
+                replace=True,
+            )
 
         self.dsc_model = (
             self.dsc_model.with_compartment_id(self.properties.compartment_id)
diff --git a/ads/model/service/oci_datascience_model_deployment.py b/ads/model/service/oci_datascience_model_deployment.py
@@ -20,10 +20,6 @@
 
 DEFAULT_WAIT_TIME = 1200
 DEFAULT_POLL_INTERVAL = 10
-DEACTIVATE_WORKFLOW_STEPS = 2
-DELETE_WORKFLOW_STEPS = 2
-ACTIVATE_WORKFLOW_STEPS = 6
-CREATE_WORKFLOW_STEPS = 6
 ALLOWED_STATUS = [
     State.ACTIVE.name,
     State.CREATING.name,
@@ -203,7 +199,6 @@ def activate(
                 try:
                     self.wait_for_progress(
                         self.workflow_req_id, 
-                        ACTIVATE_WORKFLOW_STEPS, 
                         max_wait_time, 
                         poll_interval
                     )
@@ -254,7 +249,6 @@ def create(
             try:
                 self.wait_for_progress(
                     self.workflow_req_id, 
-                    CREATE_WORKFLOW_STEPS, 
                     max_wait_time, 
                     poll_interval
                 )
@@ -317,7 +311,6 @@ def deactivate(
                 try:
                     self.wait_for_progress(
                         self.workflow_req_id, 
-                        DEACTIVATE_WORKFLOW_STEPS, 
                         max_wait_time, 
                         poll_interval
                     )
@@ -387,7 +380,6 @@ def delete(
             try:
                 self.wait_for_progress(
                     self.workflow_req_id, 
-                    DELETE_WORKFLOW_STEPS, 
                     max_wait_time, 
                     poll_interval
                 )
diff --git a/ads/opctl/utils.py b/ads/opctl/utils.py
@@ -150,11 +150,16 @@ def build_image(image_type: str, gpu: bool = False) -> None:
     else:
         # https://stackoverflow.com/questions/66842004/get-the-processor-type-using-python-for-apple-m1-processor-gives-me-an-intel-pro
         import cpuinfo
+
         # Just get the manufacturer of the processors
-        manufacturer = cpuinfo.get_cpu_info().get('brand_raw')
-        arch = 'arm' if re.search("apple m\d ", manufacturer, re.IGNORECASE) else 'other'
+        manufacturer = cpuinfo.get_cpu_info().get("brand_raw")
+        arch = (
+            "arm" if re.search("apple m\d ", manufacturer, re.IGNORECASE) else "other"
+        )
         print(f"The local machine's platform is {arch}.")
-        image, dockerfile, target = _get_image_name_dockerfile_target(image_type, gpu, arch)
+        image, dockerfile, target = _get_image_name_dockerfile_target(
+            image_type, gpu, arch
+        )
         print(f"dockerfile used is {dockerfile}")
         command = [
             "docker",
@@ -186,8 +191,6 @@ def _get_image_name_dockerfile_target(type: str, gpu: bool, arch: str) -> str:
         ("job-local", False, "arm"): (ML_JOB_IMAGE, "Dockerfile.job.arm", None),
         ("job-local", False, "other"): (ML_JOB_IMAGE, "Dockerfile.job", None),
         ("job-local", True, "other"): (ML_JOB_GPU_IMAGE, "Dockerfile.job.gpu", None),
-        ("ads-ops-base", False, "other"): (OPS_IMAGE_BASE, "Dockerfile", "base"),
-        ("ads-ops-base", True, "other"): (OPS_IMAGE_GPU_BASE, "Dockerfile.gpu", "base"),
     }
     return look_up[(type, gpu, arch)]
 
diff --git a/tests/unitary/default_setup/model_deployment/test_oci_datascience_model_deployment.py b/tests/unitary/default_setup/model_deployment/test_oci_datascience_model_deployment.py
@@ -15,10 +15,6 @@
 from ads.common.oci_mixin import OCIModelMixin, OCIWorkRequestMixin
 
 from ads.model.service.oci_datascience_model_deployment import (
-    ACTIVATE_WORKFLOW_STEPS,
-    CREATE_WORKFLOW_STEPS,
-    DEACTIVATE_WORKFLOW_STEPS,
-    DELETE_WORKFLOW_STEPS,
     OCIDataScienceModelDeployment,
 )
 
@@ -163,7 +159,6 @@ def test_activate_with_waiting(self):
                     mock_activate.assert_called_with(self.mock_model_deployment.id)
                     mock_wait.assert_called_with(
                         "test",
-                        ACTIVATE_WORKFLOW_STEPS,
                         1,
                         1,
                     )
@@ -248,7 +243,6 @@ def test_deactivate_with_waiting(self):
                     )
                     mock_wait.assert_called_with(
                         "test",
-                        DEACTIVATE_WORKFLOW_STEPS,
                         1,
                         1,
                     )
@@ -330,7 +324,6 @@ def test_create_with_waiting(self):
                                 mock_update_from_oci_model.assert_called()
                                 mock_wait.assert_called_with(
                                     "test",
-                                    CREATE_WORKFLOW_STEPS,                                    
                                     1,
                                     1,
                                 )
@@ -468,7 +461,6 @@ def test_delete_with_waiting(self):
                         mock_delete.assert_called_with(self.mock_model_deployment.id)
                         mock_wait.assert_called_with(
                             "test",
-                            DELETE_WORKFLOW_STEPS,
                             1,
                             1,
                         )
diff --git a/tests/unitary/with_extras/model/test_generic_model.py b/tests/unitary/with_extras/model/test_generic_model.py
@@ -283,19 +283,19 @@ def test_prepare_fail(self, mock_handle_model_file_name):
     @patch("ads.common.auth.default_signer")
     def test_prepare_both_conda_env(self, mock_signer, mock_get_service_packs):
         """prepare a model by only providing inference conda env."""
-        inference_conda_env="oci://service-conda-packs@ociodscdev/service_pack/cpu/General_Machine_Learning_for_CPUs/1.0/mlcpuv1"
-        inference_python_version="3.6"
-        training_conda_env="oci://service-conda-packs@ociodscdev/service_pack/cpu/Oracle_Database_for_CPU_Python_3.7/1.0/database_p37_cpu_v1"
-        training_python_version="3.7"
+        inference_conda_env = "oci://service-conda-packs@ociodscdev/service_pack/cpu/General_Machine_Learning_for_CPUs/1.0/mlcpuv1"
+        inference_python_version = "3.6"
+        training_conda_env = "oci://service-conda-packs@ociodscdev/service_pack/cpu/Oracle_Database_for_CPU_Python_3.7/1.0/database_p37_cpu_v1"
+        training_python_version = "3.7"
         mock_get_service_packs.return_value = (
             {
-                inference_conda_env : ("mlcpuv1", inference_python_version),
-                training_conda_env : ("database_p37_cpu_v1", training_python_version)
+                inference_conda_env: ("mlcpuv1", inference_python_version),
+                training_conda_env: ("database_p37_cpu_v1", training_python_version),
             },
             {
-                "mlcpuv1" : (inference_conda_env, inference_python_version),
-                "database_p37_cpu_v1" : (training_conda_env, training_python_version)
-            }
+                "mlcpuv1": (inference_conda_env, inference_python_version),
+                "database_p37_cpu_v1": (training_conda_env, training_python_version),
+            },
         )
         self.generic_model.prepare(
             inference_conda_env=inference_conda_env,
@@ -365,17 +365,19 @@ def test_reload(self):
     @patch.object(GenericModel, "_random_display_name", return_value="test_name")
     @patch.object(DataScienceModel, "create")
     @patch("ads.model.runtime.env_info.get_service_packs")
-    def test_save(self, mock_get_service_packs, mock_dsc_model_create, mock__random_display_name):
+    def test_save(
+        self, mock_get_service_packs, mock_dsc_model_create, mock__random_display_name
+    ):
         """test saving a model to artifact."""
-        inference_conda_env="oci://service-conda-packs@ociodscdev/service_pack/cpu/Data_Exploration_and_Manipulation_for_CPU_Python_3.7/3.0/dataexpl_p37_cpu_v3"
-        inference_python_version="3.7"
+        inference_conda_env = "oci://service-conda-packs@ociodscdev/service_pack/cpu/Data_Exploration_and_Manipulation_for_CPU_Python_3.7/3.0/dataexpl_p37_cpu_v3"
+        inference_python_version = "3.7"
         mock_get_service_packs.return_value = (
             {
-                inference_conda_env : ("dataexpl_p37_cpu_v3", inference_python_version),
+                inference_conda_env: ("dataexpl_p37_cpu_v3", inference_python_version),
             },
             {
-                "dataexpl_p37_cpu_v3" : (inference_conda_env, inference_python_version),
-            }
+                "dataexpl_p37_cpu_v3": (inference_conda_env, inference_python_version),
+            },
         )
         mock_dsc_model_create.return_value = MagicMock(id="fake_id")
         self.generic_model.prepare(
@@ -400,15 +402,15 @@ def test_save(self, mock_get_service_packs, mock_dsc_model_create, mock__random_
     @patch("ads.model.runtime.env_info.get_service_packs")
     def test_save_not_implemented_error(self, mock_get_service_packs):
         """test saving a model to artifact."""
-        inference_conda_env="oci://service-conda-packs@ociodscdev/service_pack/cpu/Data_Exploration_and_Manipulation_for_CPU_Python_3.7/3.0/dataexpl_p37_cpu_v3"
-        inference_python_version="3.7"
+        inference_conda_env = "oci://service-conda-packs@ociodscdev/service_pack/cpu/Data_Exploration_and_Manipulation_for_CPU_Python_3.7/3.0/dataexpl_p37_cpu_v3"
+        inference_python_version = "3.7"
         mock_get_service_packs.return_value = (
             {
-                inference_conda_env : ("dataexpl_p37_cpu_v3", inference_python_version),
+                inference_conda_env: ("dataexpl_p37_cpu_v3", inference_python_version),
             },
             {
-                "dataexpl_p37_cpu_v3" : (inference_conda_env, inference_python_version),
-            }
+                "dataexpl_p37_cpu_v3": (inference_conda_env, inference_python_version),
+            },
         )
         self.generic_model._serialize = False
         self.generic_model.prepare(
@@ -429,6 +431,40 @@ def test_save_not_implemented_error(self, mock_get_service_packs):
         with pytest.raises(NotImplementedError):
             self.generic_model.save()
 
+    @patch.object(GenericModel, "_random_display_name", return_value="test_name")
+    @patch.object(DataScienceModel, "create")
+    @patch("ads.model.runtime.env_info.get_service_packs")
+    @patch("ads.model.GenericModel.reload")
+    def test_save_not_reload(
+        self,
+        mock_reload,
+        mock_get_service_packs,
+        mock_dsc_model_create,
+        mock__random_display_name,
+    ):
+        """test saving a model to artifact without verify score.py."""
+        inference_conda_env = "oci://bucket@tenancy/prefix/dataexpl_p37_cpu_v3"
+        inference_python_version = "3.7"
+        mock_get_service_packs.return_value = (
+            {
+                inference_conda_env: ("dataexpl_p37_cpu_v3", inference_python_version),
+            },
+            {
+                "dataexpl_p37_cpu_v3": (inference_conda_env, inference_python_version),
+            },
+        )
+        mock_dsc_model_create.return_value = MagicMock(id="fake_id")
+        self.generic_model.prepare(
+            inference_conda_env="dataexpl_p37_cpu_v3",
+            namespace="ociodscdev",
+            inference_python_version="3.7",
+            model_file_name="model.joblib",
+            force_overwrite=True,
+            training_id=None,
+        )
+        self.generic_model.save(ignore_introspection=True, reload=False)
+        mock_reload.assert_not_called()
+
     def test_set_model_input_serializer(self):
         """Tests set_model_input_serializer() with different input types."""
         from ads.model.serde.model_input import (
