From 0781d269723641102e752850b04a19440416c6f2 Mon Sep 17 00:00:00 2001 From: Vinay Vinod Date: Mon, 18 Dec 2023 20:30:00 -0800 Subject: [PATCH 1/6] Only require model_id once to save a modelreq bug fix Signed-off-by: Vinay Vinod --- .../ml_models/sentencetransformermodel.py | 6 +-- .../test_sentencetransformermodel_pytest.py | 41 ++++++++++++++++++- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/opensearch_py_ml/ml_models/sentencetransformermodel.py b/opensearch_py_ml/ml_models/sentencetransformermodel.py index db5d5a226..9cd32b1af 100644 --- a/opensearch_py_ml/ml_models/sentencetransformermodel.py +++ b/opensearch_py_ml/ml_models/sentencetransformermodel.py @@ -92,7 +92,7 @@ def __init__( str("The default folder path already exists at : " + self.folder_path) ) - self.model_id = model_id + self.model_id = model_id if model_id is not None else self.DEFAULT_MODEL_ID self.torch_script_zip_file_path = None self.onnx_zip_file_path = None @@ -806,7 +806,7 @@ def save_as_pt( :rtype: string """ - model = SentenceTransformer(model_id) + model = SentenceTransformer(self.model_id) if model_name is None: model_name = str(model_id.split("/")[-1] + ".pt") @@ -1346,4 +1346,4 @@ def __is_notebook(self) -> bool: return False except NameError: # Probably standard Python interpreter - return False + return False \ No newline at end of file diff --git a/tests/ml_models/test_sentencetransformermodel_pytest.py b/tests/ml_models/test_sentencetransformermodel_pytest.py index c9c9046ba..4d08e2901 100644 --- a/tests/ml_models/test_sentencetransformermodel_pytest.py +++ b/tests/ml_models/test_sentencetransformermodel_pytest.py @@ -8,11 +8,12 @@ import json import os import shutil +from unittest.mock import MagicMock, patch from zipfile import ZipFile import pytest -from opensearch_py_ml.ml_models import SentenceTransformerModel +from opensearch_py_ml.ml_models.sentencetransformermodel import SentenceTransformerModel TEST_FOLDER = os.path.join( os.path.dirname(os.path.abspath("__file__")), "tests", "test_model_files" @@ -657,6 +658,44 @@ def test_zip_model_with_license(): clean_test_folder(TEST_FOLDER) +@pytest.fixture +def mock_opensearch_client(): + with patch("opensearchpy.OpenSearch") as mock_client: + mock_client.return_value = MagicMock() + yield mock_client + + +def test_opensearch_connection(mock_opensearch_client): + client = MLCommonClient(mock_opensearch_client) + assert client._client == mock_opensearch_client + + +def test_init(): + model = SentenceTransformerModel(model_id="test-model", folder_path="/test/folder") + assert model.folder_path == "/test/folder" + assert model.model_id == "test-model" + + +def test_train(mock_sentence_transformer, mock_file_operations): + model = SentenceTransformerModel(model_id="test-model", folder_path="/test/folder") + model.train( + read_path="dummy/path", + overwrite=True, + output_model_name="dummy_model.pt", + zip_file_name="dummy_model.zip", + num_epochs=1, + ) + # Add assertions as per your method's logic + mock_sentence_transformer.assert_called_once() + + +def test_save_as_pt_for_model(mock_sentence_transformer, mock_file_operations): + model = SentenceTransformerModel(model_id="test-model", folder_path="/test/folder") + sentences = ["This is a test sentence."] + model.save_as_pt(sentences=sentences) + + mock_sentence_transformer.return_value.save.assert_called_once() + clean_test_folder(TEST_FOLDER) clean_test_folder(TESTDATA_UNZIP_FOLDER) From 610cd3bb60a9c4f6d1c1c970958753413183a781 Mon Sep 17 00:00:00 2001 From: Vinay Vinod Date: Mon, 18 Dec 2023 20:36:07 -0800 Subject: [PATCH 2/6] Fixed linting issue with MLCommonClient import Signed-off-by: Vinay Vinod --- tests/ml_models/test_sentencetransformermodel_pytest.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/ml_models/test_sentencetransformermodel_pytest.py b/tests/ml_models/test_sentencetransformermodel_pytest.py index 4d08e2901..a52bc96d3 100644 --- a/tests/ml_models/test_sentencetransformermodel_pytest.py +++ b/tests/ml_models/test_sentencetransformermodel_pytest.py @@ -13,6 +13,7 @@ import pytest +from opensearch_py_ml.ml_commons import MLCommonClient from opensearch_py_ml.ml_models.sentencetransformermodel import SentenceTransformerModel TEST_FOLDER = os.path.join( @@ -658,6 +659,7 @@ def test_zip_model_with_license(): clean_test_folder(TEST_FOLDER) + @pytest.fixture def mock_opensearch_client(): with patch("opensearchpy.OpenSearch") as mock_client: From d22885083e03879c329b9974db5bbf253c8e76c4 Mon Sep 17 00:00:00 2001 From: Vinay Vinod Date: Mon, 18 Dec 2023 20:39:15 -0800 Subject: [PATCH 3/6] Format sentencetransformermodel.py with Black Signed-off-by: Vinay Vinod --- opensearch_py_ml/ml_models/sentencetransformermodel.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opensearch_py_ml/ml_models/sentencetransformermodel.py b/opensearch_py_ml/ml_models/sentencetransformermodel.py index 9cd32b1af..ebee6899f 100644 --- a/opensearch_py_ml/ml_models/sentencetransformermodel.py +++ b/opensearch_py_ml/ml_models/sentencetransformermodel.py @@ -1346,4 +1346,4 @@ def __is_notebook(self) -> bool: return False except NameError: # Probably standard Python interpreter - return False \ No newline at end of file + return False From 2027678f988437ae55626723abb9849d4d77b338 Mon Sep 17 00:00:00 2001 From: Vinay Vinod Date: Mon, 18 Dec 2023 20:42:40 -0800 Subject: [PATCH 4/6] Update CHANGELOG for recent changes Signed-off-by: Vinay Vinod --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4eb78bf78..535b4c5c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Add support for Model Access Control - Register, Update, Search and Delete by @rawwar in ([#332](https://github.com/opensearch-project/opensearch-py-ml/pull/332)) - Add support for model connectors by @rawwar in ([#345](https://github.com/opensearch-project/opensearch-py-ml/pull/345)) + ### Changed - Modify ml-models.JenkinsFile so that it takes model format into account and can be triggered with generic webhook by @thanawan-atc in ([#211](https://github.com/opensearch-project/opensearch-py-ml/pull/211)) - Update demo_tracing_model_torchscript_onnx.ipynb to use make_model_config_json by @thanawan-atc in ([#220](https://github.com/opensearch-project/opensearch-py-ml/pull/220)) @@ -44,6 +45,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Fix pandas dependency issue in nox session by installing pandas package to python directly by @thanawan-atc in ([#266](https://github.com/opensearch-project/opensearch-py-ml/pull/266)) - Fix conditional job execution issue in model upload workflow by @thanawan-atc in ([#294](https://github.com/opensearch-project/opensearch-py-ml/pull/294)) - fix bug in `MLCommonClient_client.upload_model` by @rawwar in ([#336](https://github.com/opensearch-project/opensearch-py-ml/pull/336)) +- Fix to ensure `model_id` is only required once when saving a model in `sentencetransformermodel.py` in ([#323](https://github.com/opensearch-project/opensearch-py-ml/pull/361)) ## [1.1.0] From 303f4855c7149915c00be4c1a646c063434a7036 Mon Sep 17 00:00:00 2001 From: Vinay Vinod Date: Mon, 18 Dec 2023 21:01:29 -0800 Subject: [PATCH 5/6] Update tests for recent changes Signed-off-by: Vinay Vinod --- .../test_sentencetransformermodel_pytest.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/tests/ml_models/test_sentencetransformermodel_pytest.py b/tests/ml_models/test_sentencetransformermodel_pytest.py index a52bc96d3..27b3cecb9 100644 --- a/tests/ml_models/test_sentencetransformermodel_pytest.py +++ b/tests/ml_models/test_sentencetransformermodel_pytest.py @@ -678,25 +678,7 @@ def test_init(): assert model.model_id == "test-model" -def test_train(mock_sentence_transformer, mock_file_operations): - model = SentenceTransformerModel(model_id="test-model", folder_path="/test/folder") - model.train( - read_path="dummy/path", - overwrite=True, - output_model_name="dummy_model.pt", - zip_file_name="dummy_model.zip", - num_epochs=1, - ) - # Add assertions as per your method's logic - mock_sentence_transformer.assert_called_once() - - -def test_save_as_pt_for_model(mock_sentence_transformer, mock_file_operations): - model = SentenceTransformerModel(model_id="test-model", folder_path="/test/folder") - sentences = ["This is a test sentence."] - model.save_as_pt(sentences=sentences) - mock_sentence_transformer.return_value.save.assert_called_once() clean_test_folder(TEST_FOLDER) From 56ecfc684978af48c524c0def0758829e50cb0ab Mon Sep 17 00:00:00 2001 From: Vinay Vinod Date: Mon, 18 Dec 2023 21:03:51 -0800 Subject: [PATCH 6/6] Update lint for recent changes Signed-off-by: Vinay Vinod --- tests/ml_models/test_sentencetransformermodel_pytest.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/ml_models/test_sentencetransformermodel_pytest.py b/tests/ml_models/test_sentencetransformermodel_pytest.py index 27b3cecb9..493b79958 100644 --- a/tests/ml_models/test_sentencetransformermodel_pytest.py +++ b/tests/ml_models/test_sentencetransformermodel_pytest.py @@ -678,8 +678,5 @@ def test_init(): assert model.model_id == "test-model" - - - clean_test_folder(TEST_FOLDER) clean_test_folder(TESTDATA_UNZIP_FOLDER)