-
Notifications
You must be signed in to change notification settings - Fork 101
🤗 HuggingFace for pretrained model weights hosting #945
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
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #945 +/- ##
===========================================
- Coverage 99.31% 99.31% -0.01%
===========================================
Files 71 71
Lines 9163 9162 -1
Branches 1196 1195 -1
===========================================
- Hits 9100 9099 -1
Misses 39 39
Partials 24 24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Are the links in the yaml file still valid? I would suggest changing these urls with |
Thanks @mostafajahanifar. This looks good to me. I have tested download of all files using test_get_pretrained_model locally and it works fine. Caching is also working fine. The only comment is to update the yaml file. If you can address this, I can merge this PR. |
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.
Pull Request Overview
This PR migrates model weight hosting from TIA servers to HuggingFace for improved distribution and caching. The changes introduce HuggingFace Hub integration while modifying default behavior to avoid automatic ImageNet weight downloads.
Key changes:
- Replace custom download logic with HuggingFace Hub for pretrained model weights
- Change default weights parameter from "DEFAULT" to None to prevent unnecessary ImageNet downloads
- Update model architecture handling for Inception and GoogleNet models
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
tiatoolbox/models/architecture/vanilla.py | Updates default weights parameter and reorganizes model architecture handling |
tiatoolbox/models/architecture/init.py | Replaces custom download with HuggingFace Hub integration |
tests/test_utils.py | Updates test to reflect new save_path behavior and adds debug output |
requirements/requirements.txt | Adds huggingface_hub dependency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@mostafajahanifar Any update on this? Please can you address the comments so we can merge this PR for the next release. |
@shaneahmed
In short, there are a few models whose name do not match the name of their checkpoints.
Personally I prefer option 1, I don't know why the checkpoint would have a different name to the model. Update: I have gone with option 1, I have renamed these two checkpoints on Hugging Face. |
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.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
tiatoolbox/models/architecture/init.py:42
- Docstring return type states Path while the function annotation returns str. Update the docstring to match the actual return type or, preferably, return a Path object and keep the docstring as Path for consistency.
Returns:
Path:
The local path to the cached pretrained weights after downloading.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
tiatoolbox/models/architecture/init.py:42
- The return type is annotated as str, but the docstring still documents Path. Please align the docstring with the actual return type (or wrap the return in Path(...) to keep the previous Path return type).
Returns:
Path:
The local path to the cached pretrained weights after downloading.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Thanks @mostafajahanifar and @Jiaqi-Lv
This PR is about moving model weights from TIA servers to HF 🤗
huggingface_hub
for downloading/caching pretrained weightsNone
modelpretrained
argument during model initialization, to avoid redundant downloading of ImageNet weights