-
Notifications
You must be signed in to change notification settings - Fork 248
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 download support for modelscople. #2032
base: master
Are you sure you want to change the base?
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Thanks for the PR! We are checking this out, and also considering whether we should just build a general method to allow downloads from different sources (e.g. register a new provider). |
Thank you. The main reason is that we are limited by the risk of the Chinese government's control over download sources from other regions. For example, hf was initially allowed to be used in China, but later we couldn't connect to it directly, and we could only use the unstable mirror sites. |
@pass-lin yes definitely! For sure we want there to be a way to use models with modelscope. And advantage of a "plugin" style api would be that people could also use this for third own private hosting (e.g. s3 bucket, or some custom remote file storage, etc). Sorry the process on this one might be slightly longer since it's adding a new (soft) dependency, but we are checking this out! |
from modelscope.hub.snapshot_download import snapshot_download | ||
except: | ||
raise ImportError( | ||
"`from_preset()` requires the `modelscope` package to " |
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.
the error message can modified as follows
"To load a preset from ModelScope {preset} using from_preset
, install the "
"modelscope
package with: pip install modelscope
."
) | ||
except HFValidationError as e: | ||
raise ValueError( | ||
"Unexpected ModelScope preset. Hugging Face model handles " |
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.
NIT: Typo - Hugging Face model handles -> ModelScope
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 the PR @pass-lin. Left some comments. As soon as they are addressed, this is ready to merge
except HFValidationError as e: | ||
raise ValueError( | ||
"Unexpected ModelScope preset. Hugging Face model handles " | ||
"should have the form 'modelscope://{org}/{model}'. For example, " |
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.
Nit: improve error message as - ModelScope handles should follow the format "
f"'modelscope://{{org}}/{{model}}' (e.g., 'modelscope://username/bert_base_en'). "
f"Received: preset='{preset}'.
Due to relevant legal restrictions of the Chinese government, Chinese users find it quite inconvenient to use HF.
Therefore, Chinese users often use the Chinese version of HF, which is ModelScope.
Considering this aspect, I think we should add download support for ModelScope.
Generally, Chinese users use it as a cloud disk for storing hf data. Therefore, we only need to make slight changes in the download section, and the other parts are exactly the same as hf.
The usage is similar to hf.