[WIP][AQUA] GPU Shape Recommendation #1221
Conversation
elizjo
requested review from
darenr,
mayoor,
mrDzurb,
VipulMascarenhas,
qiuosier and
ahosler
as code owners
oracle-contributor-agreement
bot
added
the
OCA Verified
|
📌 Cov diff with main: 📌 Overall coverage: |
|
📌 Cov diff with main: 📌 Overall coverage: |
mrDzurb
changed the title
elizjo
force-pushed
the
ODSC-74228/GPU-Shape-Recommendation
branch
from
2f54f8b to
26e08a2
Compare
|
📌 Cov diff with main: 📌 Overall coverage: |
|
📌 Cov diff with main: 📌 Overall coverage: |
|
📌 Cov diff with main: 📌 Overall coverage: |
|
📌 Cov diff with main: 📌 Overall coverage: |
|
📌 Cov diff with main: 📌 Overall coverage: |
| }, | ||
| "VM.GPU.A10.1": { | ||
| "gpu_count": 1, | ||
| "gpu_memory_in_gbs": 24, | ||
| "gpu_type": "A10" | ||
| "gpu_type": "A10", |
There was a problem hiding this comment.
Let's add FP8 for the A10 shapes as well.
ads/aqua/common/utils.py
Outdated
| @@ -1287,6 +1287,7 @@ def load_gpu_shapes_index( | |||
|
|
|||
| # Merge: remote shapes override local | |||
| local_shapes = local_data.get("shapes", {}) | |||
| remote_data = {} | |||
| @@ -13,6 +13,7 @@ | |||
| from ads.aqua.extension.evaluation_handler import __handlers__ as __eval_handlers__ | |||
| from ads.aqua.extension.finetune_handler import __handlers__ as __finetune_handlers__ | |||
| from ads.aqua.extension.model_handler import __handlers__ as __model_handlers__ | |||
| from ads.aqua.extension.recommend_handler import __handlers__ as __gpu_handlers__ | |||
There was a problem hiding this comment.
Maybe we can name it as __shape_handler?
| Detects quantization bit-width as a string (e.g., '4bit', '8bit') from Hugging Face config dict. | ||
| """ | ||
| if raw.get("load_in_8bit"): | ||
| return "8bit" |
There was a problem hiding this comment.
It would be better to move this into constants.
| If model is un-quantized, uses the weight size. | ||
| If model is pre-quantized, uses the quantization level. | ||
| """ | ||
| key = (self.quantization or self.weight_dtype or "float32").lower() |
There was a problem hiding this comment.
Let's move "float32" to constants
| """ | ||
| vals = [] | ||
| curr = min_len | ||
| max_seq_len = 16384 if not self.max_seq_len else self.max_seq_len |
There was a problem hiding this comment.
Let's move the numbers like 16384 to constants and add some description there
|
📌 Cov diff with main: 📌 Overall coverage: |
| """ | ||
|
|
||
| @handle_exceptions | ||
| def post(self, *args, **kwargs): # noqa: ARG002 |
There was a problem hiding this comment.
Would it make sense to move this handler to deployment_handler.py under the AquaDeploymentHandler class?
We already have a list_shape method there, so I suggest adding a new method called list_recommended_shape.
The endpoint path could be /aqua/deployments/recommended_shapes.
The main reason for this change is that we’ll be implementing similar logic for service models as well. Having a unified handler will allow us to return the recommended list of shapes for both service and custom models in a consistent way.
| ShapeRecommendationReport, | ||
| ShapeReport, | ||
| ) | ||
| from ads.config import COMPARTMENT_OCID |
There was a problem hiding this comment.
Looks like this const variable is not used?
| return self.rich_diff_table(shape_recommend_report) | ||
|
|
||
| @staticmethod | ||
| def validate_model_ocid(ocid: str) -> DataScienceModel: |
There was a problem hiding this comment.
If we are not planning to use this method outside of the class, it would be better to make it as private.
def _validate_model_ocid. Check for the others.
| from ads.model.datascience_model import DataScienceModel | ||
|
|
||
|
|
||
| class AquaRecommendApp(AquaApp): |
There was a problem hiding this comment.
NIT: Maybe we can name it as AquaShapeRecommendApp?
There was a problem hiding this comment.
Technically, the shape recommendation feature falls under the broader Model Deployment functionality. Instead of creating a new App class, I suggest we implement the business logic in a regular helper class and then integrate it into the existing AquaDeploymentApp.
Since AquaDeploymentApp already includes a list_shapes method, we can add a new method called list_recommended_shapes to maintain backward compatibility.
Looking ahead, if we need similar logic for other modules like fine-tuning or evaluation, we can follow the same pattern and add the corresponding methods in those apps as well.
As for naming, we could rename the current AquaRecommendApp to AquaShapeRecommend. I don't think we need to inherit from AquaApp in this case, since it's primarily a utility class focused on recommendation logic.
| Use `ads aqua recommend which_gpu --help` to get more details on available parameters. | ||
| """ | ||
|
|
||
| def which_gpu(self, **kwargs) -> ShapeRecommendationReport: |
There was a problem hiding this comment.
Maybe to make it a bit more generic - which_shape?
| return recommendations | ||
|
|
||
| @staticmethod | ||
| def rich_diff_table(shape_report: ShapeRecommendationReport) -> Table: |
| from ads.model.datascience_model import DataScienceModel | ||
|
|
||
|
|
||
| class AquaRecommendApp(AquaApp): |
There was a problem hiding this comment.
Technically, the shape recommendation feature falls under the broader Model Deployment functionality. Instead of creating a new App class, I suggest we implement the business logic in a regular helper class and then integrate it into the existing AquaDeploymentApp.
Since AquaDeploymentApp already includes a list_shapes method, we can add a new method called list_recommended_shapes to maintain backward compatibility.
Looking ahead, if we need similar logic for other modules like fine-tuning or evaluation, we can follow the same pattern and add the corresponding methods in those apps as well.
As for naming, we could rename the current AquaRecommendApp to AquaShapeRecommend. I don't think we need to inherit from AquaApp in this case, since it's primarily a utility class focused on recommendation logic.
| ValueError | ||
| If the file cannot be opened, parsed, or the 'shapes' key is missing. | ||
| """ | ||
| user_shapes = AquaDeploymentApp().list_shapes(compartment_id=compartment_id) |
There was a problem hiding this comment.
To avoid calling AquaDeploymentApp().list_shapes, I think it would be cleaner to add a shapes method directly to the OCIDataScienceModelDeployment class.
For example:
class OCIDataScienceModelDeployment(
OCIDataScienceMixin,
oci.data_science.models.ModelDeployment,
):
@classmethod
def shapes(
cls,
compartment_id: str = None,
**kwargs,
) -> List[oci.data_science.models.ModelDeploymentShapeSummary]:
return oci.pagination.list_call_get_all_results(
cls().client.list_model_deployment_shapes,
compartment_id or COMPARTMENT_ID,
**kwargs
).dataThis makes the logic more self-contained within the deployment model and avoids relying on external app instances just to retrieve shape info.
The usage could be:
user_shapes = OCIDataScienceModelDeployment.shapes(compartment_id=compartment_id)
| if name in set_user_shapes: | ||
| compute_shape = set_user_shapes.get(name) | ||
| compute_shape.available = True | ||
| compute_shape.shape_series = "GPU" |
There was a problem hiding this comment.
The oci.data_science.models.ModelDeploymentShapeSummary already contains the def shape_series(self, shape_series):. Maybe we can use it?
|
📌 Cov diff with main: 📌 Overall coverage: |
Wrote an additional POST API and aqua command for recommending GPU shapes for a particular model
Returns
Returns
Status: business logic works, API works, unit tests finished, rich diff CLI table finished
