feat: jumpstart instance specific hyperparameters #4180
Conversation
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
| default_hyperparameters[instance_specific_hyperparameter.name] = str( | ||
| instance_specific_hyperparameter.default | ||
| ) | ||
|
|
||
| return default_hyperparameters |
There was a problem hiding this comment.
nit: rename to just hyperparameters for clarity?
| ) | ||
|
|
||
| for instance_specific_hyperparameter in instance_specific_hyperparameters: | ||
| default_hyperparameters[instance_specific_hyperparameter.name] = str( |
There was a problem hiding this comment.
Would we ever want to remove a hyperparameter for a specific instance? If so, overriding the defaults may not work.
In the generated metadata, shouldn't the instance-specific hyperparameter dict contain all the fields rather than overriden ones?
There was a problem hiding this comment.
That's a good point. However, to keep consistency with the environment variable variants, I think it should be additive.
In the situation you describe, the instance family or default metadata should be scoped down so that no hyperparameters need to be removed. In other words, we can construct any arbitrary hyperparameters for instances using this metadata design.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
src/sagemaker/jumpstart/model.py
Outdated
| @@ -314,7 +314,7 @@ def _is_valid_model_id_hook(): | |||
|
|
|||
| super(JumpStartModel, self).__init__(**model_init_kwargs.to_kwargs_dict()) | |||
|
|
|||
| def retrieve_all_examples(self) -> Optional[List[JumpStartSerializablePayload]]: | |||
| def retrieve_all_example_payloads(self) -> Optional[List[JumpStartSerializablePayload]]: | |||
There was a problem hiding this comment.
is this example backward incompatible? If so, please provide an alias (as in retrieve_all_examples returns retrieve_all_example_payloads)
There was a problem hiding this comment.
sure, this was a mistake from a previous PR. I'd vote to keep as is, the risk of breaking customers is very low for this new feature. We can also deprecate the old method signature
There was a problem hiding this comment.
no please make the method name consistent with the top level utility. You can choose to remove the old method, I agree that the risk to break customers is low, but get the Python SDK to sign off on that point in that case. Alternative, use an alias for the old method and add a deprecation warn.
There was a problem hiding this comment.
Let me keep this as is for now. We can consider changing it later. This is a low priority issue
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Issue #, if available:
Description of changes:
This PR adds the capability to interpret fields in JumpStart's metadata that indicate instance specific hyperparameters. This enables JumpStart to launch models that require different hyperparameters depending on the instance type
Testing done:
Merge Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.General
Tests
unique_name_from_baseto create resource names in integ tests (if appropriate)By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.