-
Notifications
You must be signed in to change notification settings - Fork 551
Add SeedVR to model-libraries.ts #1523
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
Wauplin
left a comment
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 for opening this PR @IceClear !
I've left some comments below. I would recommend to split the repo in several ones. Also to make this metadata available in your repos, you must tag them with seedvr in the model card metadata like this:
library_name: seedvrThis will help us keep everything organized and ensure that the library is properly represented on the Hub. Thanks for your understanding! 😊
Here's the link to the page where you can check for models: https://huggingface.co/models?other=seedvr
(comment review assisted by tiny-agents)
| repoName: "SeedVR", | ||
| repoUrl: "https://github.com/ByteDance-Seed/SeedVR/", | ||
| filter: false, | ||
| countDownloads: `path_extension:"pth"`, |
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.
It is usually highly recommended to have 1 repo per model instead of 1 repo with 4 models. By doing so, you'll be able to get a download count per model and hence better analytics to understand which model is the most popular. Also, it improves discoverability for users on the Hub. And finally if some other users are interested in pushing their own models using the SeedVR library, they can also do it and follow the same repo structure as yours (which is currently not the case).
(not a obligation though, just a strong recommendation)
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.
in that case, I would rename the repo from ByteDance-Seed/SeedVR-Models to e.g. ByteDance-Seed/SeedVR2-7b for seedvr2_ema_7b.pth
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.
Hi @Wauplin I have split the repos into four following your suggestions. Kindly have a check. Thx.
Wauplin
left a comment
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 for updating @IceClear ! Looks like we have a small misunderstanding between libraries and models (see comment below) but apart from that everything looks good to me :) And thanks for splitting the repo, it looks much more canonical like this.
| "seedvr-3b": { | ||
| prettyLabel: "SeedVR-3B", | ||
| repoName: "SeedVR-3B", | ||
| repoUrl: "https://github.com/ByteDance-Seed/SeedVR/", | ||
| filter: false, | ||
| countDownloads: `path_extension:"pth"`, | ||
| }, | ||
| "seedvr-7b": { | ||
| prettyLabel: "SeedVR-7B", | ||
| repoName: "SeedVR-7B", | ||
| repoUrl: "https://github.com/ByteDance-Seed/SeedVR/", | ||
| filter: false, | ||
| countDownloads: `path_extension:"pth"`, | ||
| }, | ||
| "seedvr2-3b": { | ||
| prettyLabel: "SeedVR2-3B", | ||
| repoName: "SeedVR2-3B", | ||
| repoUrl: "https://github.com/ByteDance-Seed/SeedVR/", | ||
| filter: false, | ||
| countDownloads: `path_extension:"pth"`, | ||
| }, | ||
| "seedvr2-7b": { | ||
| prettyLabel: "SeedVR2-7B", | ||
| repoName: "SeedVR2-7B", | ||
| repoUrl: "https://github.com/ByteDance-Seed/SeedVR/", | ||
| filter: false, | ||
| countDownloads: `path_extension:"pth"`, | ||
| }, |
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.
| "seedvr-3b": { | |
| prettyLabel: "SeedVR-3B", | |
| repoName: "SeedVR-3B", | |
| repoUrl: "https://github.com/ByteDance-Seed/SeedVR/", | |
| filter: false, | |
| countDownloads: `path_extension:"pth"`, | |
| }, | |
| "seedvr-7b": { | |
| prettyLabel: "SeedVR-7B", | |
| repoName: "SeedVR-7B", | |
| repoUrl: "https://github.com/ByteDance-Seed/SeedVR/", | |
| filter: false, | |
| countDownloads: `path_extension:"pth"`, | |
| }, | |
| "seedvr2-3b": { | |
| prettyLabel: "SeedVR2-3B", | |
| repoName: "SeedVR2-3B", | |
| repoUrl: "https://github.com/ByteDance-Seed/SeedVR/", | |
| filter: false, | |
| countDownloads: `path_extension:"pth"`, | |
| }, | |
| "seedvr2-7b": { | |
| prettyLabel: "SeedVR2-7B", | |
| repoName: "SeedVR2-7B", | |
| repoUrl: "https://github.com/ByteDance-Seed/SeedVR/", | |
| filter: false, | |
| countDownloads: `path_extension:"pth"`, | |
| }, | |
| "seedvr": { | |
| prettyLabel: "SeedVR", | |
| repoName: "SeedVR", | |
| repoUrl: "https://github.com/ByteDance-Seed/SeedVR", | |
| filter: false, | |
| countDownloads: `path_extension:"pth"`, | |
| }, |
Small clarification: in this PR we are adding metadata for a library (SeedVR) that can be used with several models (the 4 from https://huggingface.co/models?other=seedvr). So there's no need to register 1 library per repo, simply 1 for seedvr tag and that's it.
Apart from that, looks like we are ready to merge!
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 a lot for your clarification @Wauplin ! Done
Wauplin
left a comment
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.
Perfect! Let's get this merged then :)
Once merged, expect 2-3 days delay before getting it to production. Download count will be retroactive.
This MR is for registering the SeedVR model to the hub
https://huggingface.co/ByteDance-Seed/SeedVR-Models