Skip to content

Accept container family as input for model creation and param validation #862

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 22, 2024

Conversation

VipulMascarenhas
Copy link
Member

Description

This PR includes the changes for:

  1. When user registers the model and inference container is not set, then set default as odsc-tgi-serving.
  2. When deployment is created, accept container_family as input when user deploys unverified models.
  3. When model params are being validated, accept container_family as input when user validates params for unverified models.

Unit Tests

> python -m pytest -q tests/unitary/with_extras/aqua/test_deployment*.py
==================================================== test session starts =====================================================
platform darwin -- Python 3.8.18, pytest-7.4.0, pluggy-1.0.0
rootdir: /Users/user/workspace/git/accelerated-data-science
configfile: pytest.ini
plugins: Faker-24.9.0, anyio-4.2.0
collected 29 items

tests/unitary/with_extras/aqua/test_deployment.py ....................                                                 [ 68%]
tests/unitary/with_extras/aqua/test_deployment_handler.py ..s......                                                    [100%]

=============================================== 28 passed, 1 skipped in 7.69s ================================================

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label May 21, 2024
)

if not inference_container:
inference_container = InferenceContainerTypeKey.AQUA_TGI_CONTAINER_KEY
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be based on the check if this model has text_generation_inference tag. If not, we should default to vllm

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated this to get the right image family based on the text_generation_inference tag. I thought all text-generation models will have the tag anyway, so didn't add an additional check here.

@@ -40,6 +40,25 @@ class Tags(str, metaclass=ExtendedEnumMeta):
AQUA_EVALUATION_MODEL_ID = "evaluation_model_id"


class InferenceContainerType(str, metaclass=ExtendedEnumMeta):
Copy link
Member Author

@VipulMascarenhas VipulMascarenhas May 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi - moved this from datasciencemodeldeployment.enums to here to avoid circular imports.

)
logger.debug(message)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: shouldn't we log here the actual error, not the one that we generated?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not for this one. We want this to be a user friendly message when this entry is not available in custom metadata.

@VipulMascarenhas VipulMascarenhas merged commit 9d1a67b into feature/aquav1.0.2 May 22, 2024
@VipulMascarenhas VipulMascarenhas deleted the aqua_container_image_updates branch May 22, 2024 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants