-
Notifications
You must be signed in to change notification settings - Fork 227
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
chore: add Moshi #914
chore: add Moshi #914
Conversation
repoName: "Moshi", | ||
repoUrl: "https://github.com/kyutai-labs/moshi", | ||
filter: false, | ||
countDownloads: `path:"tokenizer-e351c8d8-checkpoint125.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.
this looks super specific no?
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.
Yes! Since there's a mic of safetensors
and GGUFs
this points to the tokenizer for now - this is the same across all the repos, irrespective of the backend (candle, mlx, PyTorch) - it's the safest way I found without double counting.
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.
Any thoughts about having
countDownloads: `path:"tokenizer-e351c8d8-checkpoint125.safetensors"`, | |
countDownloads: `path:"model.safetensors" OR path:"model.q4.safetensors" OR path:"model.q8.safetensors" OR path:"model.gguf" OR path:"model.q4.gguf" OR path:"model.q8.gguf"`, |
?
It's long and ugly but at least if a new tokenizer is pushed, it won't break the download count. And it wouldn't duplicate counts if I checked correctly.
Otherwise I'm fine with the current solution, there is no perfect one anyway.
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.
should we add snippets?
Yes! I was thinking about it, but wasn't sure how best to support all three backends, since each has a different setup and installation 😅 |
if i'm not mistaken there's a way to specify multiple snippets now, even with titles/collapsible sections etc |
can be in a separate PR though, if you prefer |
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.
pre-approving just in case
Is it possible to add (agree can be done in a follow-up 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.
Thanks for handling it @Vaibhavs10 :)
Approving "as is" (comments above are optional)
Cool! ah yeah! Good shout re: MLX + Candle tags, I'll add them tomorrow/ later today and make a follow-up PR 🤗 Thank you for the review and suggestions! ❤️ |
ref: https://huggingface.co/models?other=moshi