Skip to content

Conversation

@haithamkhedr
Copy link

Track downloads for sam3 repo

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.

Quickly reviewed it, though we won't shipped it until sam3 repo is live (with sam3 in the model card metadata).

Comment on lines +968 to +969
prettyLabel: "sam3",
repoName: "sam3",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
prettyLabel: "sam3",
repoName: "sam3",
prettyLabel: "SAM 3",
repoName: "SAM 3",

(maybe?)

repoName: "sam3",
repoUrl: "https://github.com/facebookresearch/sam3",
filter: false,
countDownloads: `path:"config.json" OR path_extension:"pt"`,
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the repo file structure exactly? I expect the config.json file to be required to run the model, correct? If that's the case, then I would count downloads only on this file. The problem with counting both on config.json and weights files .pt is that we would be double-counting downloads.

So what I'm suggesting is to count only on config.json (same as for transformers models), which happens to be the default rule to count downloads, meaning we can just remove the rule and it will work out of the box:

Suggested change
countDownloads: `path:"config.json" OR path_extension:"pt"`,

Copy link
Member

@pcuenca pcuenca Nov 19, 2025

Choose a reason for hiding this comment

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

The repo structure is expected to be like SAM2: https://huggingface.co/facebook/sam2-hiera-large/tree/main. The problem is that it hosts weights for two libraries: the original code (backed by the pt file), and the transformers weights.

The goal with this PR was to question whether it is possible to count downloads for both. More specifically, if we set library_name to sam3, and add transformers as a tag:

  • Will download counts work as expected? (I think so)
  • Will the "Use this model" in transformers work? (Yes)
  • Will the "Use this model" in colab/Kaggle create a transformers notebook? Not sure; I can check the code in our internal repo to verify. As an alternative, we may want to include a notebook.ipynb for this use case.

Are there any other features that you think could be affected by this setup?

Copy link
Member

Choose a reason for hiding this comment

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

(And some of these changes may need to be backported to the SAM2 repos too)

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, didn't see the related thread in time! Then my suggestion would be to either:

  • download the config.json in the SAM3 library even if not used (adds a very little overhead on first load but that's all) => if you go for this solution, no need to register a custom download count rule
  • or set the download count rule to
Suggested change
countDownloads: `path:"config.json" OR path_extension:"pt"`,
countDownloads: `path:"config.json" OR path:"sam3.pt"`,

if we know exactly what will be the pt filename.

@Wauplin
Copy link
Contributor

Wauplin commented Nov 19, 2025

(discussed offline and this PR is not needed)

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.

4 participants