-
Notifications
You must be signed in to change notification settings - Fork 60
Changed multi model metadata storage. #1112
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
Changed multi model metadata storage. #1112
Conversation
…racle/accelerated-data-science into ODSC-68579/improved_multi_model_metadata_storage_2
ads/aqua/constants.py
Outdated
| # Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl/ | ||
| """This module defines constants used in ads.aqua module.""" | ||
|
|
||
| UNKNOWN = "" |
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.
Let's use it form common.utils instead
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.
Updated
| ModelCustomMetadataItem(key=ModelCustomMetadataFields.MULTIMODEL_METADATA), | ||
| ).value | ||
|
|
||
| if not multi_model_metadata_value: |
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.
error_message = (
f"Required model metadata is missing for evaluation source ID: {create_aqua_evaluation_details.evaluation_source_id}. "
f"A valid multi-model deployment requires {ModelCustomMetadataFields.MULTIMODEL_METADATA}. "
"Please recreate the model deployment and retry the evaluation, as an issue occurred during the initialization of the model group."
)
logger.debug(error_message)
raise AquaRuntimeError(error_message)
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.
Updated
ads/aqua/evaluation/evaluation.py
Outdated
| logger.debug(error_message) | ||
| raise AquaRuntimeError(error_message) | ||
|
|
||
| multi_model_metadata = json.loads( |
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.
Let's add more debug statements, indicating the steps we are doing here.
I also think that we should put the loading the metadata into try/catch to prevent returning useless error messages to the users. For users we should say that something wrong with loading the metadata, and put details into the debug logs.
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.
Updated
| ) | ||
|
|
||
| # Build the list of valid model names from custom metadata. | ||
| model_names = [ |
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.
We should load metadata back to the AquaMultiModelRef
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.
Updated
| # Finalize creation | ||
| custom_model.create(model_by_reference=True) | ||
|
|
||
| # Create custom metadata for multi model metadata |
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.
let's add more debug statements here
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.
Updated
| ), | ||
| ).value | ||
| if not multi_model_metadata_value: | ||
| raise AquaRuntimeError( |
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.
raise AquaRuntimeError(
f"Invalid multi-model deployment: {model_deployment_id}. "
f"Ensure that the required custom metadata `{ModelCustomMetadataFields.MULTIMODEL_METADATA}` is added to the AQUA multi-model `{aqua_model.display_name}` ({aqua_model.id})."
)
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.
Updated
Improved multi model metadata storage
create_custom_metadata_artifactto store multi model metadata in model catalog while creating the aqua model.get_custom_metadata_artifactto get multi model metadata in model catalog while fetching the aqua deployment.Notebook