Skip to content

Conversation

elizjo
Copy link
Member

@elizjo elizjo commented Sep 12, 2025

For AQUA Service models, pre-defined configuration files already exist and can be directly used. This PR adds support for service models via obtaining their pre-defined config files, parsing this metadata, and formatting a response consistent with the current output of the GPU Shape Recommendation Tool

ads aqua deployment recommend_shape --model_id  'ocid1.datasciencemodel.oc1.<ocid>' 

Returns
Screenshot 2025-09-12 at 12 17 48 PM

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Sep 12, 2025
mrDzurb
mrDzurb previously approved these changes Sep 12, 2025
Copy link

📌 Cov diff with main:

Coverage-0%

📌 Overall coverage:

Coverage-18.23%

Copy link

📌 Cov diff with main:

Coverage-78%

📌 Overall coverage:

Coverage-58.41%

Copy link
Member

@VipulMascarenhas VipulMascarenhas left a comment

Choose a reason for hiding this comment

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

minor comments. overall looks good. 👍

If model type is unsupported by tool (no recommendation report generated)
If the model type is unsupported and no recommendation report can be generated.
"""
deployment_config = self.get_deployment_config(model_id=kwargs.get("model_id"))
Copy link
Member

Choose a reason for hiding this comment

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

nit: add a validation here to check if kwargs has model id before passing to get_deployment_config.

"""
c = self.llm_config
embedding_count = 1 if getattr(c, "tie_word_embeddings", True) else 2
llm_config = self.llm_config
Copy link
Member

@VipulMascarenhas VipulMascarenhas Sep 16, 2025

Choose a reason for hiding this comment

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

nit: we could just do self.llm_config below instead of declaring a new var, just personal preference. We can keep it as is for now.

@elizjo elizjo merged commit ed095fd into main Sep 16, 2025
23 checks passed
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.

3 participants