-
Notifications
You must be signed in to change notification settings - Fork 360
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
Add judgment for whether model_id is a local path to speed up local model loading #523
Conversation
Code Metrics Report=============================================================================== Language Files Lines Code Comments Blanks =============================================================================== Dockerfile 1 34 25 0 9 Happy 1 442 369 0 73 JSON 10 66 65 0 1 Python 36 1412 1220 38 154 TOML 18 495 446 2 47 ------------------------------------------------------------------------------- Jupyter Notebooks 1 0 0 0 0 |- Markdown 1 60 30 22 8 |- Python 1 96 87 1 8 (Total) 156 117 23 16 ------------------------------------------------------------------------------- Markdown 21 1551 0 1171 380 |- BASH 5 101 98 0 3 |- JSON 1 12 12 0 0 |- Python 5 98 88 0 10 |- Rust 4 221 198 10 13 |- TOML 2 75 63 0 12 (Total) 2058 459 1181 418 ------------------------------------------------------------------------------- Rust 131 44155 40029 779 3347 |- Markdown 77 723 13 671 39 (Total) 44878 40042 1450 3386 =============================================================================== Total 220 48155 42154 1990 4011 =============================================================================== |
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.
Thank you for adding this! I think that if we check that the file exists, this may be more flexible. Is there a reason why you chose to use is_aboslute
rather than exists
?
Hhh just not sure whether it would break some old logics. And If it is not, we can just check if model_id exists as a local path |
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.
Thank you! We keep the logic for file path checking in these macros, so this should be fine.
In the original logic of marcos.os, regardless of whether model_id is a valid ID on Hugging Face, a request to Hugging Face is made, and if the request fails, it then attempts to load from the local environment. However, if the network environment cannot connect to Hugging Face normally, this step becomes very slow, with each API call attempt taking tens of seconds. Therefore, I have added a workaround here: first, it checks whether model_id is an absolute path. If it is, it directly loads from the local environment. This simple modification directly reduces the time spent on load_model_from_hf from 1 to 2 minutes to just a few microseconds 🤣
Edit: Now I change to check if model_id exists as a local path