Skip to content
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

Updated MedVAE Download Counts #1245

Merged
merged 10 commits into from
Mar 5, 2025
Merged

Updated MedVAE Download Counts #1245

merged 10 commits into from
Mar 5, 2025

Conversation

ashwinkumargb
Copy link
Contributor

Hi, thanks for updating the download counts for Merlin. I'm trying to do something similar for MedVAE. The files are under a folder called model_weights, but I don't think it matters for path_extension since filepaths are inherently flattened. Please let me know if there are any issues!

Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ashwinkumargb , thanks for the PR :) Left a comment regarding the download count rule. Having the files in a subfolder is indeed not problematic. However we usually recommend to store 1 model weight per repo instead of having a weights folder with different versions in it. This way you would be able to have a downloadcount "per variant" and not globally. This is only a suggestion on how to structure your models/repos for the library.

In that case if you want to avoid maintaining multiple model cards you can make them all refer to a single repo.

@ashwinkumargb
Copy link
Contributor Author

Hi @Wauplin, thanks for the quick action! That makes sense to only count ckpt files to avoid double counting!

Got it, I figured one model per repo is the norm and will keep it in mind going forward. In this case, we were thinking of advertising MedVAE as an abstraction for all the models, so thought it would be easier with the huggingface model repo to advertise in the same way. Also, we will be adding more model in future to the same repo and thought this would be easier.

@ashwinkumargb
Copy link
Contributor Author

@Wauplin Just following up on this PR!

Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for updating :)

Re: multiple models in same repo: Ok for now but keep in mind you can't have granular metrics by doing so.

I'll merge this PR now. Expect 3-4 days before getting analytics live on production

@Wauplin Wauplin merged commit 4b0963d into huggingface:main Mar 5, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants