From c6e3a699a9a226719cd84d86057431323c2de31e Mon Sep 17 00:00:00 2001 From: Sara Robinson Date: Fri, 26 Aug 2022 15:12:25 -0400 Subject: [PATCH 1/4] feat: add model versioning support to Model.get_model_evaluation and list_model_evaluations --- google/cloud/aiplatform/models.py | 32 +++++++++++++++-- tests/unit/aiplatform/test_models.py | 52 +++++++++++++++++++++++++++- 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/google/cloud/aiplatform/models.py b/google/cloud/aiplatform/models.py index 796fb5fd66..7ed0381fb8 100644 --- a/google/cloud/aiplatform/models.py +++ b/google/cloud/aiplatform/models.py @@ -4569,8 +4569,12 @@ def upload_tensorflow_saved_model( def list_model_evaluations( self, + version: Optional[str] = None, + list_all_evaluations: Optional[bool] = False, ) -> List["model_evaluation.ModelEvaluation"]: """List all Model Evaluation resources associated with this model. + If no version is provided, Model Evaluation resources will be returned + for the default version. Example Usage: my_model = Model( @@ -4579,21 +4583,41 @@ def list_model_evaluations( my_evaluations = my_model.list_model_evaluations() + OR + + my_version_evaluations = my_model.list_model_evaluations(version="2") + + version (str): + Optional. A model version ID or alias to return Model Evaluations for. + list_all_evaluations (bool): + Optional. If set to True, returns Model Evaluations from all versions of this model. Defaults to False. + Returns: List[model_evaluation.ModelEvaluation]: List of ModelEvaluation resources for the model. """ - self.wait() + if list_all_evaluations and version: + raise ValueError( + "If `list_all_evaluations` is set to True, do not pass `version` to list_model_evaluations()." + ) + + if list_all_evaluations: + model_resource_name = f"{self.resource_name}@-" + else: + model_resource_name = ModelRegistry._get_versioned_name( + self.resource_name, version=version + ) return model_evaluation.ModelEvaluation._list( - parent=self.resource_name, + parent=model_resource_name, credentials=self.credentials, ) def get_model_evaluation( self, evaluation_id: Optional[str] = None, + model_version_id: Optional[str] = None, ) -> Optional[model_evaluation.ModelEvaluation]: """Returns a ModelEvaluation resource and instantiates its representation. If no evaluation_id is passed, it will return the first evaluation associated @@ -4614,13 +4638,15 @@ def get_model_evaluation( Args: evaluation_id (str): Optional. The ID of the model evaluation to retrieve. + model_version_id (str): + Optional. The ID of the model version to retrieve the evaluation from. Returns: model_evaluation.ModelEvaluation: Instantiated representation of the ModelEvaluation resource. """ - evaluations = self.list_model_evaluations() + evaluations = self.list_model_evaluations(version=model_version_id) if not evaluation_id: if len(evaluations) > 1: diff --git a/tests/unit/aiplatform/test_models.py b/tests/unit/aiplatform/test_models.py index e71d257fc0..f8b01a980c 100644 --- a/tests/unit/aiplatform/test_models.py +++ b/tests/unit/aiplatform/test_models.py @@ -2357,7 +2357,7 @@ def test_update(self, update_model_mock, get_model_mock): model=current_model_proto, update_mask=update_mask ) - def test_get_model_evaluation_with_id( + def test_get_model_evaluation_with_evaluation_id( self, mock_model_eval_get, get_model_mock, @@ -2371,6 +2371,26 @@ def test_get_model_evaluation_with_id( name=_TEST_MODEL_EVAL_RESOURCE_NAME, retry=base._DEFAULT_RETRY ) + def test_get_model_evaluation_with_evaluation_and_model_version_id( + self, + mock_model_eval_get, + get_model_mock, + list_model_evaluations_mock, + ): + test_model = models.Model(model_name=_TEST_MODEL_RESOURCE_NAME) + + test_model.get_model_evaluation( + evaluation_id=_TEST_ID, model_version_id=_TEST_VERSION_ID + ) + + mock_model_eval_get.assert_called_once_with( + name=_TEST_MODEL_EVAL_RESOURCE_NAME, retry=base._DEFAULT_RETRY + ) + + list_model_evaluations_mock.assert_called_once_with( + request={"parent": f"{_TEST_MODEL_RESOURCE_NAME}@{_TEST_VERSION_ID}"} + ) + def test_get_model_evaluation_without_id( self, mock_model_eval_get, @@ -2402,6 +2422,36 @@ def test_list_model_evaluations( assert len(eval_list) == len(_TEST_MODEL_EVAL_LIST) + def test_list_model_evaluations_with_version( + self, + get_model_mock, + mock_model_eval_get, + list_model_evaluations_mock, + ): + + test_model = models.Model(model_name=_TEST_MODEL_RESOURCE_NAME) + + eval_list = test_model.list_model_evaluations(version=_TEST_VERSION_ID) + + list_model_evaluations_mock.assert_called_once_with( + request={"parent": f"{_TEST_MODEL_RESOURCE_NAME}@{_TEST_VERSION_ID}"} + ) + + def test_list_model_evaluations_for_all_versions( + self, + get_model_mock, + mock_model_eval_get, + list_model_evaluations_mock, + ): + + test_model = models.Model(model_name=_TEST_MODEL_RESOURCE_NAME) + + eval_list = test_model.list_model_evaluations(list_all_evaluations=True) + + list_model_evaluations_mock.assert_called_once_with( + request={"parent": f"{_TEST_MODEL_RESOURCE_NAME}@-"} + ) + def test_init_with_version_in_resource_name(self, get_model_with_version): model = models.Model( model_name=models.ModelRegistry._get_versioned_name( From 9e4d960d5cf65526b75a39e1a63dddfa4201a469 Mon Sep 17 00:00:00 2001 From: Sara Robinson Date: Fri, 26 Aug 2022 17:24:37 -0400 Subject: [PATCH 2/4] linting fix --- tests/unit/aiplatform/test_models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/aiplatform/test_models.py b/tests/unit/aiplatform/test_models.py index f8b01a980c..550b628620 100644 --- a/tests/unit/aiplatform/test_models.py +++ b/tests/unit/aiplatform/test_models.py @@ -2431,7 +2431,7 @@ def test_list_model_evaluations_with_version( test_model = models.Model(model_name=_TEST_MODEL_RESOURCE_NAME) - eval_list = test_model.list_model_evaluations(version=_TEST_VERSION_ID) + test_model.list_model_evaluations(version=_TEST_VERSION_ID) list_model_evaluations_mock.assert_called_once_with( request={"parent": f"{_TEST_MODEL_RESOURCE_NAME}@{_TEST_VERSION_ID}"} @@ -2446,7 +2446,7 @@ def test_list_model_evaluations_for_all_versions( test_model = models.Model(model_name=_TEST_MODEL_RESOURCE_NAME) - eval_list = test_model.list_model_evaluations(list_all_evaluations=True) + test_model.list_model_evaluations(list_all_evaluations=True) list_model_evaluations_mock.assert_called_once_with( request={"parent": f"{_TEST_MODEL_RESOURCE_NAME}@-"} From fee8408dde35740b712969578cc9cbf6e994ae3f Mon Sep 17 00:00:00 2001 From: Sara Robinson Date: Wed, 31 Aug 2022 09:08:09 -0400 Subject: [PATCH 3/4] update list_model_evaluations to use instantiated model version --- google/cloud/aiplatform/models.py | 33 +++++----------------------- tests/unit/aiplatform/test_models.py | 21 ++++-------------- 2 files changed, 10 insertions(+), 44 deletions(-) diff --git a/google/cloud/aiplatform/models.py b/google/cloud/aiplatform/models.py index 7ed0381fb8..c1e6e71235 100644 --- a/google/cloud/aiplatform/models.py +++ b/google/cloud/aiplatform/models.py @@ -4569,48 +4569,27 @@ def upload_tensorflow_saved_model( def list_model_evaluations( self, - version: Optional[str] = None, - list_all_evaluations: Optional[bool] = False, ) -> List["model_evaluation.ModelEvaluation"]: """List all Model Evaluation resources associated with this model. - If no version is provided, Model Evaluation resources will be returned - for the default version. + If this Model resource was instantiated with a version, the Model + Evaluation resources for that version will be returned. If no version + was provided when the Model resource was instantiated, Model Evaluation + resources will be returned for the default version. Example Usage: my_model = Model( - model_name="projects/123/locations/us-central1/models/456" + model_name="projects/123/locations/us-central1/models/456@1" ) my_evaluations = my_model.list_model_evaluations() - OR - - my_version_evaluations = my_model.list_model_evaluations(version="2") - - version (str): - Optional. A model version ID or alias to return Model Evaluations for. - list_all_evaluations (bool): - Optional. If set to True, returns Model Evaluations from all versions of this model. Defaults to False. - Returns: List[model_evaluation.ModelEvaluation]: List of ModelEvaluation resources for the model. """ - if list_all_evaluations and version: - raise ValueError( - "If `list_all_evaluations` is set to True, do not pass `version` to list_model_evaluations()." - ) - - if list_all_evaluations: - model_resource_name = f"{self.resource_name}@-" - else: - model_resource_name = ModelRegistry._get_versioned_name( - self.resource_name, version=version - ) - return model_evaluation.ModelEvaluation._list( - parent=model_resource_name, + parent=self.versioned_resource_name, credentials=self.credentials, ) diff --git a/tests/unit/aiplatform/test_models.py b/tests/unit/aiplatform/test_models.py index 550b628620..d685651c00 100644 --- a/tests/unit/aiplatform/test_models.py +++ b/tests/unit/aiplatform/test_models.py @@ -2429,27 +2429,14 @@ def test_list_model_evaluations_with_version( list_model_evaluations_mock, ): - test_model = models.Model(model_name=_TEST_MODEL_RESOURCE_NAME) - - test_model.list_model_evaluations(version=_TEST_VERSION_ID) - - list_model_evaluations_mock.assert_called_once_with( - request={"parent": f"{_TEST_MODEL_RESOURCE_NAME}@{_TEST_VERSION_ID}"} + test_model = models.Model( + model_name=f"{_TEST_MODEL_RESOURCE_NAME}@{_TEST_VERSION_ID}" ) - def test_list_model_evaluations_for_all_versions( - self, - get_model_mock, - mock_model_eval_get, - list_model_evaluations_mock, - ): - - test_model = models.Model(model_name=_TEST_MODEL_RESOURCE_NAME) - - test_model.list_model_evaluations(list_all_evaluations=True) + test_model.list_model_evaluations() list_model_evaluations_mock.assert_called_once_with( - request={"parent": f"{_TEST_MODEL_RESOURCE_NAME}@-"} + request={"parent": test_model.versioned_resource_name} ) def test_init_with_version_in_resource_name(self, get_model_with_version): From 0e8df0e6b69dd68eb550ffd42260c4bacaaf22d9 Mon Sep 17 00:00:00 2001 From: Sara Robinson Date: Wed, 31 Aug 2022 09:20:34 -0400 Subject: [PATCH 4/4] update get evaluation method --- google/cloud/aiplatform/models.py | 10 +++++----- tests/unit/aiplatform/test_models.py | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/google/cloud/aiplatform/models.py b/google/cloud/aiplatform/models.py index c1e6e71235..201dfb66a9 100644 --- a/google/cloud/aiplatform/models.py +++ b/google/cloud/aiplatform/models.py @@ -4596,11 +4596,13 @@ def list_model_evaluations( def get_model_evaluation( self, evaluation_id: Optional[str] = None, - model_version_id: Optional[str] = None, ) -> Optional[model_evaluation.ModelEvaluation]: """Returns a ModelEvaluation resource and instantiates its representation. If no evaluation_id is passed, it will return the first evaluation associated - with this model. + with this model. If the aiplatform.Model resource was instantiated with a + version, this will return a Model Evaluation from that version. If no version + was specified when instantiating the Model resource, this will return an + Evaluation from the default version. Example usage: my_model = Model( @@ -4617,15 +4619,13 @@ def get_model_evaluation( Args: evaluation_id (str): Optional. The ID of the model evaluation to retrieve. - model_version_id (str): - Optional. The ID of the model version to retrieve the evaluation from. Returns: model_evaluation.ModelEvaluation: Instantiated representation of the ModelEvaluation resource. """ - evaluations = self.list_model_evaluations(version=model_version_id) + evaluations = self.list_model_evaluations() if not evaluation_id: if len(evaluations) > 1: diff --git a/tests/unit/aiplatform/test_models.py b/tests/unit/aiplatform/test_models.py index d685651c00..f81cc3fe7f 100644 --- a/tests/unit/aiplatform/test_models.py +++ b/tests/unit/aiplatform/test_models.py @@ -2371,24 +2371,24 @@ def test_get_model_evaluation_with_evaluation_id( name=_TEST_MODEL_EVAL_RESOURCE_NAME, retry=base._DEFAULT_RETRY ) - def test_get_model_evaluation_with_evaluation_and_model_version_id( + def test_get_model_evaluation_with_evaluation_and_instantiated_version( self, mock_model_eval_get, get_model_mock, list_model_evaluations_mock, ): - test_model = models.Model(model_name=_TEST_MODEL_RESOURCE_NAME) - - test_model.get_model_evaluation( - evaluation_id=_TEST_ID, model_version_id=_TEST_VERSION_ID + test_model = models.Model( + model_name=f"{_TEST_MODEL_RESOURCE_NAME}@{_TEST_VERSION_ID}" ) + test_model.get_model_evaluation(evaluation_id=_TEST_ID) + mock_model_eval_get.assert_called_once_with( name=_TEST_MODEL_EVAL_RESOURCE_NAME, retry=base._DEFAULT_RETRY ) list_model_evaluations_mock.assert_called_once_with( - request={"parent": f"{_TEST_MODEL_RESOURCE_NAME}@{_TEST_VERSION_ID}"} + request={"parent": test_model.versioned_resource_name} ) def test_get_model_evaluation_without_id(