-
Notifications
You must be signed in to change notification settings - Fork 48
[AQUA] Adding ADS support for embedding models in Multi Model Deployment #1163
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
Changes from 4 commits
4248b98
11802a7
c1a0478
4e2bda0
7897b38
987b60d
f37c6e0
e0bbc9b
46a6789
44c25b8
d6b66f1
c21d4cb
2bb7a9f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -80,7 +80,7 @@ | |
ImportModelDetails, | ||
ModelValidationResult, | ||
) | ||
from ads.aqua.model.enums import MultiModelSupportedTaskType | ||
from ads.aqua.model.enums import MultiModelSupportedTaskType, MultiModelConfigMode | ||
from ads.common.auth import default_signer | ||
from ads.common.oci_resource import SEARCH_TYPE, OCIResource | ||
from ads.common.utils import ( | ||
|
@@ -316,6 +316,11 @@ def create_multi( | |
|
||
display_name_list.append(display_name) | ||
|
||
model_task = source_model.freeform_tags.get(Tags.TASK, UNKNOWN) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would rather to move this logic to the
I believe we should also allow users to pass task within There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we allow user to pass task, or if not provided, use the freeform tags of the source model |
||
|
||
if model_task != UNKNOWN: | ||
self._get_task(model, model_task) | ||
|
||
# Retrieve model artifact | ||
model_artifact_path = source_model.artifact | ||
if not model_artifact_path: | ||
|
@@ -704,6 +709,15 @@ def edit_registered_model( | |
else: | ||
raise AquaRuntimeError("Only registered unverified models can be edited.") | ||
|
||
def _get_task( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? |
||
self, | ||
model: AquaMultiModelRef, | ||
freeform_task_tag: str | ||
) -> str: | ||
"""In a Multi Model Deployment, will set model task if freeform task tag from model needs a non-completion endpoint (embedding)""" | ||
if freeform_task_tag == MultiModelSupportedTaskType.EMBEDDING_ALT: | ||
model.model_task = MultiModelConfigMode.EMBEDDING | ||
|
||
def _fetch_metric_from_metadata( | ||
self, | ||
custom_metadata_list: ModelCustomMetadata, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we add embedding as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed- we add embedding in SMC level