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

Add Sana .pth files into download counting in huggingface repo #1061

Merged
merged 12 commits into from
Dec 5, 2024

Conversation

lawrence-cj
Copy link
Contributor

No description provided.

Copy link
Member

@Vaibhavs10 Vaibhavs10 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @lawrence-cj left 2 quick suggestions!

packages/tasks/src/model-libraries.ts Outdated Show resolved Hide resolved
packages/tasks/src/model-libraries-snippets.ts Outdated Show resolved Hide resolved
@lawrence-cj
Copy link
Contributor Author

Cool!

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.

Hey there, thanks for the PR and the back and forth with @Vaibhavs10 :) I left 2 comments and then we should be good to go.

packages/tasks/src/model-libraries-snippets.ts Outdated Show resolved Hide resolved
prettyLabel: "Sana",
repoName: "Sana",
repoUrl: "https://github.com/NVlabs/Sana",
countDownloads: `path_extension:"pth" OR path_extension:"json"`,
Copy link
Contributor

Choose a reason for hiding this comment

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

.pth file each time the pipeline is instantiated. Is that the case or not? And is the config.json ever loaded from script?

  1. If the script only loads the .pth file, then let's count only on .pth:
Suggested change
countDownloads: `path_extension:"pth" OR path_extension:"json"`,
countDownloads: `path_extension:"pth"`,
  1. If the script loads the config file as well, then it's even better and we should count these calls and only these calls. config.json is already tracked by default and that's why some count are already listed on the Hub (see https://huggingface.co/Efficient-Large-Model/Sana_1600M_512px). In that case, no need to add a countDownloads rule.
Suggested change
countDownloads: `path_extension:"pth" OR path_extension:"json"`,

I let you decide what's best for Sana library.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, we didn't have a config.json file until yesterday. We want to recover it, so we add pth here. Is this reasonable?

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, makes sense to track pth files then 👍

@lawrence-cj lawrence-cj requested a review from Wauplin December 5, 2024 06:18
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!

@Wauplin
Copy link
Contributor

Wauplin commented Dec 5, 2024

Lint is finally passing 😄
I'm merging. Expect 2-3 days before getting it deployed to production.

@Wauplin Wauplin merged commit 690e143 into huggingface:main Dec 5, 2024
4 checks passed
@lawrence-cj
Copy link
Contributor Author

OHH, Great to know. Thanks a lot for your support and quick apply @Wauplin @Vaibhavs10

@lawrence-cj
Copy link
Contributor Author

Hi experts. Sorry for interrupting. The downloading count does not recover in our collection. For example, https://huggingface.co/Efficient-Large-Model/Sana_1600M_1024px
Could you pls help to check the problem?
Cc: @Wauplin @Vaibhavs10

@Wauplin
Copy link
Contributor

Wauplin commented Dec 10, 2024

Hi @lawrence-cj , sorry about the delay. The release did not make it to production yet. I've opened an internal PR to make it happen.

@Wauplin
Copy link
Contributor

Wauplin commented Dec 10, 2024

Hey @lawrence-cj I can confirm to you that this PR has been deployed to production. I can now see 7400+ downloads for https://huggingface.co/Efficient-Large-Model/Sana_1600M_1024px for instance. Is that more in line with what you were expecting?

@lawrence-cj
Copy link
Contributor Author

Oh, thank you so much for it @Wauplin. Actually, I would have expected more as we have 70K downloads of our component models. But that's ok, I'm ok if that's the backend data.

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.

3 participants