[ODSC-63984] BYOC TEI deployment for embedding models #975

VipulMascarenhas · 2024-10-18T23:23:07Z

Description

This update adds the ability to use BYOC for deploying an embedding model using custom TEI container. The changes include:

While registering the embedding model, the custom metadata will have a new cmd var entry with the details required to start the TEI container.
During registration, user can pass the image uri using inference_container_uri parameter.
During deployment, two params are added: container_image_uri and cmd_var
- If model has the default image as TEI in custom metadata, then container_image_uri will be used from metadata. If passed as input param during deployment, the input container_image_uri will be used instead.
- If model has the default image as TEI in custom metadata, cmd_var will be fetched from the metadata as well. User can add additional parameters if needed. We can add some additional restriction on parameters that can be passed as cmd_var if needed.

Registering a model

ads aqua model register --model_id sentence-transformers/all-MiniLM-L6-v2 --os_path oci://<bucket>@<namespace>/<prefix> --download_from_hf True --inference_container odsc-tei-serving --inference_container_uri <region>.ocir.io/<namespace>/text-embeddings-inference:1.5.0

Deploying a model

With GPU shape

ads aqua deployment create --model_id ocid1.datasciencemodel.oc1.iad.<ocid> --instance_shape VM.GPU.A10.1 --display_name "sentence-transformers/all-MiniLM-L6-v2 MD 1017" --env_var "{'MODEL_DEPLOY_PREDICT_ENDPOINT': '/v1/embeddings'}"

With GPU shape and CMD override

ads aqua deployment create --model_id ocid1.datasciencemodel.oc1.iad.<ocid> --instance_shape VM.GPU.A10.1 --display_name "sentence-transformers/all-MiniLM-L6-v2 MD 1017" --env_var "{'MODEL_DEPLOY_PREDICT_ENDPOINT': '/v1/embeddings'}" --cmd_var "['--max-concurrent-requests', 256, '--auto-truncate', '--payload-limit', 2000000]"

Validation for restricted keys for CMD

ads aqua deployment create --model_id ocid1.datasciencemodel.oc1.iad.<ocid> --instance_shape VM.GPU.A10.1 --display_name "sentence-transformers/all-MiniLM-L6-v2 MD 1017" --env_var "{'MODEL_DEPLOY_PREDICT_ENDPOINT': '/v1/embeddings'}"--cmd_var "['--model-id', 'test-model']"

ERROR:ads:The following keys cannot be overridden: --model-id

With CPU shape

Here, registered model already had the default container URI set to GPU container, but user can override using container_image_uri params.

ads aqua deployment create --model_id ocid1.datasciencemodel.oc1.iad.<ocid> --instance_shape VM.Standard.E4.Flex --display_name "sentence-transformers/all-MiniLM-L6-v2 MD 1017" --env_var "{'MODEL_DEPLOY_PREDICT_ENDPOINT': '/v1/embeddings'}" --container_image_uri iad.ocir.io/ociodscdev/text-embeddings-inference:cpu-1.5

Unit Tests

> python -m pytest -q tests/unitary/with_extras/aqua/*
========================================== test session starts ===========================================
platform darwin -- Python 3.8.19, pytest-7.4.4, pluggy-1.0.0
rootdir: /Users/user/workspace/git/accelerated-data-science
configfile: pytest.ini
plugins: Faker-26.0.0, cov-5.0.0, anyio-4.2.0, xdist-3.6.1
collected 248 items

tests/unitary/with_extras/aqua/test_cli.py ............................                            [  5%]
tests/unitary/with_extras/aqua/test_common_handler.py ...                                          [  6%]
tests/unitary/with_extras/aqua/test_config.py .                                                    [  7%]
tests/unitary/with_extras/aqua/test_decorator.py ..........                                        [ 11%]
tests/unitary/with_extras/aqua/test_deployment.py ......................                           [ 20%]
tests/unitary/with_extras/aqua/test_deployment_handler.py ..s......                                [ 23%]
tests/unitary/with_extras/aqua/test_evaluation.py .............................                    [ 35%]
tests/unitary/with_extras/aqua/test_evaluation_handler.py .........                                [ 39%]
tests/unitary/with_extras/aqua/test_evaluation_service_config.py .....................             [ 47%]
tests/unitary/with_extras/aqua/test_finetuning.py .......                                          [ 50%]
tests/unitary/with_extras/aqua/test_finetuning_handler.py .....                                    [ 52%]
tests/unitary/with_extras/aqua/test_global.py ...                                                  [ 53%]
tests/unitary/with_extras/aqua/test_handlers.py .................                                  [ 60%]
tests/unitary/with_extras/aqua/test_model.py ..........................                            [ 70%]
tests/unitary/with_extras/aqua/test_model_handler.py ..............                                [ 76%]
tests/unitary/with_extras/aqua/test_ui.py ...............                                          [ 82%]
tests/unitary/with_extras/aqua/test_ui_handler.py ..............                                   [ 88%]
tests/unitary/with_extras/aqua/test_ui_websocket_handler.py ....                                   [ 89%]
tests/unitary/with_extras/aqua/test_utils.py ...........

==================================== 247 passed, 1 skipped in 50.04s =====================================

mrDzurb · 2024-10-18T23:31:01Z

docs/source/release_notes.rst

+-------
+Release date: October 18, 2024
+
+* Introduced enhancements for AI Quick Actions.


I think it would be nice to mention which enhancement.

github-actions · 2024-10-18T23:54:07Z

📌 Cov diff with main:

📌 Overall coverage:

github-actions · 2024-10-23T23:55:14Z

📌 Cov diff with main:

📌 Overall coverage:

github-actions · 2024-10-24T07:09:57Z

📌 Cov diff with main:

📌 Overall coverage:

mayoor · 2024-10-24T17:28:13Z

ads/aqua/modeldeployment/deployment.py

+            logging.info(f"CMD used for deploying {aqua_model.id} :{cmd_var}")
+        else:
+            # fetch image name from config
+            container_image_uri = get_container_image(container_type=container_type_key)


Can we drop this if statement and instead follow this logic -

get_container_image returns empty when the image is not found

If it is empty, we check if aqua_model.custom_metadata_list.get(AQUA_DEPLOYMENT_CONTAINER_URI_METADATA_NAME).value has value.

If both are empty we return value error

yes, dropped the TEI specific check and updated to the above logic.

github-actions · 2024-10-24T21:10:07Z

📌 Cov diff with main:

📌 Overall coverage:

github-actions · 2024-10-25T00:36:19Z

📌 Cov diff with main:

📌 Overall coverage:

ads/aqua/common/utils.py

mrDzurb · 2024-10-25T00:56:31Z

ads/aqua/common/utils.py

+    """Helper functions that parses a list into a key-value dictionary. The list contains keys separated by the prefix
+    '--' and the value of the key is the subsequent element.
+    """
+    it = iter(cmd_list)


github-actions · 2024-10-26T00:19:07Z

📌 Cov diff with main:

📌 Overall coverage:

github-actions · 2024-10-28T08:28:34Z

📌 Cov diff with main:

📌 Overall coverage:

mayoor · 2024-10-29T01:20:39Z

ads/aqua/modeldeployment/deployment.py

+            cmd_var_string = aqua_model.custom_metadata_list.get(
+                AQUA_DEPLOYMENT_CONTAINER_CMD_VAR_METADATA_NAME
+            ).value
+            default_cmd_var = cmd_var_string.split(",")


we can use shlex.split for safety since this is a cmd string

using shlex.split now instead of string split.

mayoor · 2024-10-29T01:23:34Z

ads/aqua/common/utils.py

+    """
+    it = iter(cmd_list)
+    return {
+        key: next(it, None) if not key.startswith("--") else next(it, None)


since the filter on the for loop already takes in key that starts --, do we need to check it again. Also, both if and else, we return the same, so we dont need the if statement right?

I reverted to the iterative method, this one made me think twice as well. Easier for readability for the future.

mayoor · 2024-10-29T01:25:41Z

ads/aqua/model/model.py

+                description=f"Inference container mapping for {model_name}",
+                category="Other",
+            )
+            if (


What will happen once we have SMC support? The user will see this warning unless they upgrade the sdk right? Let us get rid of this check. There is already check in the upstream code right?

removed this check

mayoor · 2024-10-29T01:29:41Z

ads/aqua/model/model.py

+                        f"Proceeding with model registration without the inference container URI for "
+                        f"{inference_container}. You can still add this configuration during model deployment."
+                    )
+                else:


we can add

if inference_container_uri: metadata.add( key=AQUA_DEPLOYMENT_CONTAINER_URI_METADATA_NAME, value=inference_container_uri, description=f"Inference container URI for {model_name}", category="Other", )

for the following part, can we have a generate only when the config.json does not any information about the TEI? When we support SMC, we might have backward compatible issue -

cmd_vars = generate_tei_cmd_var(os_path)

now we add this to metadata only if the container image family is not in the SMC list.

github-actions · 2024-10-29T17:52:51Z

📌 Cov diff with main:

📌 Overall coverage:

kumar-shivam-ranjan

LGTM

VipulMascarenhas added 6 commits October 16, 2024 14:11

add minor aqua features

356cd3a

updated release notes and version

458d0cd

register and deployment changes for byoc

22ae99c

update cmd var

0ab772a

clean up

679d44f

merge conflict

ee8be98

VipulMascarenhas requested review from darenr, mayoor, mrDzurb, qiuosier and ahosler as code owners October 18, 2024 23:23

oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Oct 18, 2024

VipulMascarenhas changed the base branch from main to feature/aqua-v1.0.5 October 18, 2024 23:29

VipulMascarenhas changed the base branch from feature/aqua-v1.0.5 to main October 18, 2024 23:29

VipulMascarenhas changed the title ~~[ODSC-63984] BYOC TEI image for embedding models~~ WIP - [ODSC-63984] BYOC TEI image for embedding models Oct 18, 2024

VipulMascarenhas changed the title ~~WIP - [ODSC-63984] BYOC TEI image for embedding models~~ WIP - [ODSC-63984] BYOC TEI deployment for embedding models Oct 18, 2024

mrDzurb previously approved these changes Oct 18, 2024

View reviewed changes

VipulMascarenhas added 2 commits October 23, 2024 15:44

changes for tei byoc deployment

2b5cd84

fix tests

e080822

VipulMascarenhas dismissed mrDzurb’s stale review via e080822 October 23, 2024 23:16

VipulMascarenhas changed the title ~~WIP - [ODSC-63984] BYOC TEI deployment for embedding models~~ [ODSC-63984] BYOC TEI deployment for embedding models Oct 23, 2024

merge conflicts

be72643

VipulMascarenhas requested a review from kumar-shivam-ranjan October 23, 2024 23:28

mayoor reviewed Oct 24, 2024

View reviewed changes

review comments

07483da

temp skip test

168b4f8

mrDzurb previously approved these changes Oct 25, 2024

View reviewed changes

VipulMascarenhas added 2 commits October 25, 2024 15:37

clear cache before after run

b918ebc

resolve merge conflicts and ruff updates

fb863b7

VipulMascarenhas dismissed mrDzurb’s stale review via fb863b7 October 25, 2024 22:42

revert unit tests for debugging failures

5e30a01

fix model tests

4ccd311

mrDzurb previously approved these changes Oct 28, 2024

View reviewed changes

mayoor reviewed Oct 29, 2024

View reviewed changes

VipulMascarenhas added 4 commits October 28, 2024 22:34

Support backward compatibility for TEI register command

b6dbc5b

Replace string split with shlex split

80cb16c

Simplify cmd parsing logic

7c7d774

Resolve merge conflicts with main

cf81e28

VipulMascarenhas dismissed mrDzurb’s stale review via cf81e28 October 29, 2024 17:24

mayoor approved these changes Oct 29, 2024

View reviewed changes

kumar-shivam-ranjan approved these changes Oct 29, 2024

View reviewed changes

VipulMascarenhas merged commit 4936025 into main Oct 29, 2024
22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ODSC-63984] BYOC TEI deployment for embedding models #975

[ODSC-63984] BYOC TEI deployment for embedding models #975

VipulMascarenhas commented Oct 18, 2024 •

edited

Loading

mrDzurb Oct 18, 2024

github-actions bot commented Oct 18, 2024

github-actions bot commented Oct 23, 2024

github-actions bot commented Oct 24, 2024

mayoor Oct 24, 2024

VipulMascarenhas Oct 24, 2024

github-actions bot commented Oct 24, 2024

github-actions bot commented Oct 25, 2024

mrDzurb Oct 25, 2024

github-actions bot commented Oct 26, 2024

github-actions bot commented Oct 28, 2024

mayoor Oct 29, 2024

VipulMascarenhas Oct 29, 2024

mayoor Oct 29, 2024

VipulMascarenhas Oct 29, 2024

mayoor Oct 29, 2024

VipulMascarenhas Oct 29, 2024

mayoor Oct 29, 2024 •

edited

Loading

VipulMascarenhas Oct 29, 2024

github-actions bot commented Oct 29, 2024

kumar-shivam-ranjan left a comment

[ODSC-63984] BYOC TEI deployment for embedding models #975

[ODSC-63984] BYOC TEI deployment for embedding models #975

Conversation

VipulMascarenhas commented Oct 18, 2024 • edited Loading

Description

Registering a model

Deploying a model

With GPU shape

With GPU shape and CMD override

Validation for restricted keys for CMD

With CPU shape

Unit Tests

Choose a reason for hiding this comment

github-actions bot commented Oct 18, 2024

github-actions bot commented Oct 23, 2024

github-actions bot commented Oct 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 24, 2024

github-actions bot commented Oct 25, 2024

Choose a reason for hiding this comment

github-actions bot commented Oct 26, 2024

github-actions bot commented Oct 28, 2024

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

mayoor Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 29, 2024

kumar-shivam-ranjan left a comment

Choose a reason for hiding this comment

VipulMascarenhas commented Oct 18, 2024 •

edited

Loading

mayoor Oct 29, 2024 •

edited

Loading