[AQUA] Adding ADS support for embedding models in Multi Model Deployment #1163

elizjo · 2025-04-23T01:59:04Z

This PR adds support of using embedding models in a multi model deployment.

To accomplish this, we have to pass data specifying the model task from ADS to the MULTI_MODEL_CONFIG environment variable in model deployment.

##Before PR
MULTI_MODEL_CONFIG=
'{ "models": [ 
{ "params": "--served-model-name bge 
--tensor-parallel-size 1 
--trust-remote-code 
--max-model-len 4096", 
"model_path": "bge-m3" } , { "params": "--served-model-name llama --enforce-eager --max-num-seqs 16 --tensor-parallel-size 2 --max-model-len 16000", "model_path": "Llama-3.2-11B-Vision" }] }'

##After PR- see 'model_task' key
MULTI_MODEL_CONFIG=
'{ "models": [  
{ "params": "--served-model-name bge
 --tensor-parallel-size 1 
--trust-remote-code 
--max-model-len 4096", 
"model_path": "bge-m3", 
"model_task": "embedding" }, { "params": "--served-model-name llama --enforce-eager --max-num-seqs 16 --tensor-parallel-size 2 --max-model-len 16000", "model_path": "Llama-3.2-11B-Vision" }, ] }'

We only have 'model_task' key for embedding models used in a multi model deployment.

We extract whether a model is an embedding model by reading the freeform tags ('task' tag) of the model (to determine whether model is an embedding model)
We added model_task as an optional parameter in the AquaMultiModelRef object
the model_task parameter is used to construct the MULTI_MODEL_CONFIG which has added (model_task: "embedding") key.

All unit tests pass (see screenshot). This PR was tested by modifying the existing unit test test_create_deployment_for_multi_model.

github-actions · 2025-04-23T02:31:44Z

📌 Cov diff with main:

📌 Overall coverage:

github-actions · 2025-04-23T17:57:15Z

📌 Cov diff with main:

📌 Overall coverage:

mrDzurb · 2025-04-23T17:55:20Z

ads/aqua/model/enums.py

@@ -28,3 +28,8 @@ class FineTuningCustomMetadata(ExtendedEnum):
 class MultiModelSupportedTaskType(ExtendedEnum):
    TEXT_GENERATION = "text-generation"
    TEXT_GENERATION_ALT = "text_generation"
+    EMBEDDING_ALT = "text_embedding"


Shouldn't we add embedding as well?

fixed- we add embedding in SMC level

mrDzurb · 2025-04-23T20:57:36Z

ads/aqua/model/model.py

@@ -316,6 +316,11 @@ def create_multi(

            display_name_list.append(display_name)

+            model_task = source_model.freeform_tags.get(Tags.TASK, UNKNOWN)


I would rather to move this logic to the _get_task() method

model.taks = self._get_task(model, source_model)

def _get_task(model_ref:AquaMultiModelRef, source_model: DataScienceModel) -> str: # extract task from model_ref by itself, if task is not presented there, then extract it from the freeform tags. # model_task = source_model.freeform_tags.get(Tags.TASK, UNKNOWN) .... return taks

I believe we should also allow users to pass task within AquaMultiModelRef, just in case if the tags were not populated well.

we allow user to pass task, or if not provided, use the freeform tags of the source model

github-actions · 2025-04-23T23:46:48Z

📌 Cov diff with main:

📌 Overall coverage:

github-actions · 2025-04-24T17:28:35Z

📌 Cov diff with main:

📌 Overall coverage:

mrDzurb · 2025-04-24T17:59:15Z

ads/aqua/model/model.py

+            model.model_task = task_tag
+        else:
+            raise AquaValueError(
+                f"{task_tag} is not supported. Valid model_task inputs are: {MultiModelSupportedTaskType.values()}."


In case of empty task_tag, what the error will look like?

mrDzurb · 2025-04-24T18:04:20Z

ads/aqua/model/model.py

@@ -707,6 +700,25 @@ def edit_registered_model(
        else:
            raise AquaRuntimeError("Only registered unverified models can be edited.")

+    def _get_task(


Looks like this method doesn't return any value, yet its signature indicates a return type of str. Should we update the type hint to reflect that it returns None, or adjust the implementation to return a string as specified?

mrDzurb · 2025-04-24T18:06:45Z

ads/aqua/model/model.py

            display_name_list.append(display_name)

+            self._get_task(model, source_model)


I think It might be clearer if we do something like this:

model.model_task = self._extract_model_task(model, source_model)

mrDzurb · 2025-04-24T18:16:08Z

ads/aqua/model/model.py

+        if task_tag in MultiModelSupportedTaskType:
+            model.model_task = task_tag
+        else:
+            raise AquaValueError(


We can show more informative error:

raise AquaValueError( f"Invalid or missing {task_tag} tag for selected model {display_name}. " f"Currently only `{MultiModelSupportedTaskType.values()}` models are supported for multi model deployment."

since we removed the task level validation in the recent release, any reason to add the validation in the function `_extract_model_task again?

This is fine for now since we only have 1 verified embedding model, but if in the future if we start supporting (unverified) models, embedding models could have task value as feature_extraction or sentence_similarity. Might be good to add a comment here to reconsider this logic when we start supporting additional models.

github-actions · 2025-04-24T19:20:10Z

📌 Cov diff with main:

📌 Overall coverage:

VipulMascarenhas

minor comment, overall looks good.

VipulMascarenhas · 2025-04-24T20:42:59Z

ads/aqua/model/model.py

+        if task_tag in MultiModelSupportedTaskType:
+            model.model_task = task_tag
+        else:
+            raise AquaValueError(


since we removed the task level validation in the recent release, any reason to add the validation in the function `_extract_model_task again?

This is fine for now since we only have 1 verified embedding model, but if in the future if we start supporting (unverified) models, embedding models could have task value as feature_extraction or sentence_similarity. Might be good to add a comment here to reconsider this logic when we start supporting additional models.

github-actions · 2025-04-24T22:24:08Z

📌 Cov diff with main:

📌 Overall coverage:

elizjo added 2 commits April 21, 2025 13:49

added support for embedding models in multi model

4248b98

fixed unit tests for embedding multi model deployments

11802a7

elizjo requested review from darenr, mayoor, mrDzurb, VipulMascarenhas, qiuosier and ahosler as code owners April 23, 2025 01:59

oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Apr 23, 2025

removed print statements and added clarifying docstring

c1a0478

Merge branch 'main' into ODSC-70052/embedding_multi_model

4e2bda0

mrDzurb changed the title ~~Adding ADS support for embedding models in Multi Model Deployment~~ [AQUA] Adding ADS support for embedding models in Multi Model Deployment Apr 23, 2025

mrDzurb reviewed Apr 23, 2025

View reviewed changes

added validation logic for model_task and unit test in test_model.py

7897b38

elizjo added 4 commits April 24, 2025 09:47

refactored _get_task

987b60d

removed comment and added case-insensitive match for model_task

f37c6e0

updated comment on model_task pydantic parameter description

e0bbc9b

Merge branch 'main' into ODSC-70052/embedding_multi_model

46a6789

mrDzurb reviewed Apr 24, 2025

View reviewed changes

elizjo added 3 commits April 24, 2025 11:16

fixed type signature and changed method name

44c25b8

updated error messages

d6b66f1

fixed diff for model.py

c21d4cb

mrDzurb previously approved these changes Apr 24, 2025

View reviewed changes

VipulMascarenhas previously approved these changes Apr 24, 2025

View reviewed changes

added comment about revisiting logic

2bb7a9f

elizjo dismissed stale reviews from VipulMascarenhas and mrDzurb via 2bb7a9f April 24, 2025 21:54

mrDzurb approved these changes Apr 24, 2025

View reviewed changes

VipulMascarenhas approved these changes Apr 24, 2025

View reviewed changes

elizjo merged commit 8d7b9d5 into main Apr 25, 2025
23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AQUA] Adding ADS support for embedding models in Multi Model Deployment #1163

[AQUA] Adding ADS support for embedding models in Multi Model Deployment #1163

elizjo commented Apr 23, 2025 •

edited

Loading

github-actions bot commented Apr 23, 2025

github-actions bot commented Apr 23, 2025

mrDzurb Apr 23, 2025

elizjo Apr 24, 2025

mrDzurb Apr 23, 2025

elizjo Apr 24, 2025

github-actions bot commented Apr 23, 2025

github-actions bot commented Apr 24, 2025

mrDzurb Apr 24, 2025

mrDzurb Apr 24, 2025

mrDzurb Apr 24, 2025

elizjo Apr 24, 2025

mrDzurb Apr 24, 2025

elizjo Apr 24, 2025

VipulMascarenhas Apr 24, 2025

github-actions bot commented Apr 24, 2025

VipulMascarenhas left a comment

VipulMascarenhas Apr 24, 2025

github-actions bot commented Apr 24, 2025

		@@ -316,6 +316,11 @@ def create_multi(

		display_name_list.append(display_name)

		model_task = source_model.freeform_tags.get(Tags.TASK, UNKNOWN)

		display_name_list.append(display_name)

		self._get_task(model, source_model)

[AQUA] Adding ADS support for embedding models in Multi Model Deployment #1163

[AQUA] Adding ADS support for embedding models in Multi Model Deployment #1163

Conversation

elizjo commented Apr 23, 2025 • edited Loading

github-actions bot commented Apr 23, 2025

github-actions bot commented Apr 23, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Apr 23, 2025

github-actions bot commented Apr 24, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Apr 24, 2025

VipulMascarenhas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Apr 24, 2025

elizjo commented Apr 23, 2025 •

edited

Loading