feat: jumpstart model id suggestions#2899
feat: jumpstart model id suggestions#2899navinsoni merged 12 commits intoaws:devaws/sagemaker-python-sdk:devfrom evakravi:feat/jumpstart-model-id-suggestionsevakravi/sagemaker-python-sdk:feat/jumpstart-model-id-suggestionsCopy head branch name to clipboard
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #2899 +/- ##
=======================================
Coverage 89.47% 89.48%
=======================================
Files 196 196
Lines 16584 16592 +8
=======================================
+ Hits 14839 14847 +8
Misses 1745 1745
Continue to review full report at Codecov.
|
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 |
96c01d7 to
c437191
Compare
JGuinegagne
left a comment
There was a problem hiding this comment.
Minor comments/doc changes.
Also a question on how many model IDs suggestion do we surface.
src/sagemaker/jumpstart/cache.py
Outdated
| ) | ||
| if other_model_id_version is not None: | ||
| error_msg += ( | ||
| f"Consider using model id {model_id} with version " |
There was a problem hiding this comment.
"Consider using model ID '{model_id}' with version '{other_model_id_version}'."
|
|
||
| else: | ||
| possible_model_ids = [header.model_id for header in manifest.values()] | ||
| closest_model_id = get_close_matches(model_id, possible_model_ids, n=1, cutoff=0)[0] |
There was a problem hiding this comment.
question: should you show only one? or say for example: at least 1, up to 3 when score > xx ?
There was a problem hiding this comment.
Let's just do 1 and make it simple. If we want to make this really good, we're better off just having a separate utility for searching model ids.
There was a problem hiding this comment.
another question, can you get an IndexError here if get_close_matches returns an empty list?
If that's possible, could you handle this edge case and add a unit test for it?
There was a problem hiding this comment.
No, because the cutoff is 0, there will always be a match.
There was a problem hiding this comment.
Thanks. I know this is super edge case, but could there be a case where possible_model_ids is an empty list?
There was a problem hiding this comment.
Only if the manifest is empty
| error_msg = f"Unable to find model manifest for {model_id} with version {version}. " | ||
|
|
||
| other_model_id_version = self._select_version( | ||
| "*", versions_incompatible_with_sagemaker |
There was a problem hiding this comment.
can you please add a comment as to why you use this variable.
There was a problem hiding this comment.
reiterate: please explain why we use a variable called versions_incompatitble_with_sagemaker here.
| """This module defines the JumpStartModelsCache class.""" | ||
| from __future__ import absolute_import | ||
| import datetime | ||
| from difflib import get_close_matches |
049a57c to
4aa1670
Compare
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 |
4aa1670 to
d66432e
Compare
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 |
d66432e to
c51d9bd
Compare
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 |
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 |
Co-authored-by: Navin Soni <navinsoni89@gmail.com> Co-authored-by: Mufaddal Rohawala <89424143+mufaddal-rohawala@users.noreply.github.com> Co-authored-by: qidewenwhen <32910701+qidewenwhen@users.noreply.github.com> Co-authored-by: HappyAmazonian <91216626+HappyAmazonian@users.noreply.github.com>
Co-authored-by: Navin Soni <navinsoni89@gmail.com> Co-authored-by: Mufaddal Rohawala <89424143+mufaddal-rohawala@users.noreply.github.com> Co-authored-by: qidewenwhen <32910701+qidewenwhen@users.noreply.github.com> Co-authored-by: HappyAmazonian <91216626+HappyAmazonian@users.noreply.github.com>
Issue #, if available:
Description of changes:
This PR improves the error message when a bad model id is inputted for JumpStart utilities. If the model id does not exist for any versions, the error message indicates a closest match model id which can be used.
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.