-
Notifications
You must be signed in to change notification settings - Fork 432
Add univa to models #1526
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
base: main
Are you sure you want to change the base?
Add univa to models #1526
Conversation
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.
LGTM! requires the tag to be merged on the hub before merge: https://huggingface.co/LanguageBind/UniWorld-V1/blob/main/README.md?code=true#L3
"univa": { | ||
prettyLabel: "univa", | ||
repoName: "univa", |
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.
"univa": { | |
prettyLabel: "univa", | |
repoName: "univa", | |
"uniworld": { | |
prettyLabel: "UniWorld", | |
repoName: "UniWorld", |
Shouldn't the library be named UniWorld
instead to follow the repo naming convention? I feel it would be more natural that way
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 library itself was called univa, i.e. import univa, the model is loaded from there. if the authors choose to train new models loaded with this code it would be more unified imo
@@ -1027,6 +1027,14 @@ export const MODEL_LIBRARIES_UI_ELEMENTS = { | |||
countDownloads: `path_extension:"pt"`, | |||
snippets: snippets.ultralytics, | |||
}, | |||
"univa": { |
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.
as mentioned by @Vaibhavs10 let's get the model listed under https://huggingface.co/models?other=uniworld (or https://huggingface.co/models?other=univa) before shipping 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.
before this PR @NielsRogge has opened this PR https://huggingface.co/LanguageBind/UniWorld-V1/discussions/3/files they haven't responded, pinged them again
repoUrl: "https://github.com/PKU-YuanGroup/UniWorld-V1", | ||
snippets: snippets.univa, | ||
filter: true, | ||
countDownloads: `path_extension:"safetensors"`, |
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.
Looks like the repo already have a config.json
file so would be good to keep the counting on that file (if loaded at each .from_pretrained
). This is the default rule if we don't set one.
Problem with counting path_extension:"safetensors"
is that it's non-canonical + will count 17 downloads each time a user downloads the repo (given the 17 shards). (taken from files section)
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.
fixed this, thanks for the heads up!
Co-authored-by: vb <vaibhavs10@gmail.com>
Co-authored-by: vb <vaibhavs10@gmail.com>
@Vaibhavs10 @linoytsaban as discussed