-
Notifications
You must be signed in to change notification settings - Fork 978
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
Implement Huggingface model download in storage initializer #3584
base: master
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: andyi2it The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
0d2d1af
to
7f95e45
Compare
7f95e45
to
34a548a
Compare
@@ -272,8 +271,28 @@ def _download_s3(uri, temp_dir: str): | |||
if mimetype in ["application/x-tar", "application/zip"]: | |||
Storage._unpack_archive_file(target, mimetype, temp_dir) | |||
|
|||
@staticmethod | |||
def _download_hf(uri, temp_dir: str): | |||
from transformers import AutoTokenizer, AutoConfig, AutoModel |
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.
instead of loading model and then saving the model to local, which means you need resources (lot of memory) to load the model, maybe snapshot_download SDK will be better.
@@ -24,6 +24,7 @@ env: | |||
# Controller images | |||
CONTROLLER_IMG: "kserve-controller" | |||
STORAGE_INIT_IMG: "storage-initializer" | |||
HF_STORAGE_INIT_IMG: "hf-storage-initializer" |
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.
why have a new image here? I assume the feature will be in the storage initializer and we should use the single image for all types of storage.
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.
@lizzzcai
The problem is storage initializer image size grows significantly to support huggingface server, from 500+ Mb to 1.2+ Gb approximately. That would be fairly large increase in image size for a feature only a few subset of users may use.
So, we decided to use a separate image for huggingface server.
Signed-off-by: Andrews Arokiam <andrews.arokiam@ideas2it.com>
Signed-off-by: Andrews Arokiam <andrews.arokiam@ideas2it.com>
Signed-off-by: Andrews Arokiam <andrews.arokiam@ideas2it.com>
Signed-off-by: Andrews Arokiam <andrews.arokiam@ideas2it.com>
Signed-off-by: Andrews Arokiam <andrews.arokiam@ideas2it.com>
Signed-off-by: Andrews Arokiam <andrews.arokiam@ideas2it.com>
Signed-off-by: Andrews Arokiam <andrews.arokiam@ideas2it.com>
Signed-off-by: Andrews Arokiam <andrews.arokiam@ideas2it.com>
Signed-off-by: Andrews Arokiam <andrews.arokiam@ideas2it.com>
Signed-off-by: Andrews Arokiam <andrews.arokiam@ideas2it.com>
Signed-off-by: Andrews Arokiam <andrews.arokiam@ideas2it.com>
Signed-off-by: Andrews Arokiam <andrews.arokiam@ideas2it.com>
Signed-off-by: Andrews Arokiam <andrews.arokiam@ideas2it.com>
Signed-off-by: Andrews Arokiam <andrews.arokiam@ideas2it.com>
Signed-off-by: Andrews Arokiam <andrews.arokiam@ideas2it.com>
34a548a
to
e2779b4
Compare
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #3545
Type of changes
Please delete options that are not relevant.
Feature/Issue validation/testing:
Please describe the tests that you ran to verify your changes and relevant result summary. Provide instructions so it can be reproduced.
Please also list any relevant details for your test configuration.
Test A
Test B
Logs
Special notes for your reviewer:
Checklist:
Release note: