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 TabPFN as library #1094

Merged
merged 4 commits into from
Jan 10, 2025
Merged

Add TabPFN as library #1094

merged 4 commits into from
Jan 10, 2025

Conversation

merveenoyan
Copy link
Contributor

The authors of TabPFN want to track the model (it's being downloaded a lot on GH it seems but not as much on Hub because they track config) @pcuenca

@merveenoyan
Copy link
Contributor Author

@pcuenca can you briefly review?

Copy link
Member

@pcuenca pcuenca left a comment

Choose a reason for hiding this comment

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

Thank you! Could you maybe open a PR to their github to show how to download from the Hub?

repoName: "TabPFN",
repoUrl: "https://github.com/PriorLabs/TabPFN",
filter: false,
countDownloads: `path_extension:"ckpt"`,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
countDownloads: `path_extension:"ckpt"`,
countDownloads: `path_extension:"ckpt" or path:"config.json"`,

maybe?

Copy link
Contributor

@Wauplin Wauplin Jan 13, 2025

Choose a reason for hiding this comment

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

@merveenoyan @pcuenca Sorry bringing this back up on a closed PR but doing this will double-count downloads since both the ckpt and the config.json files are downloaded (or attempted to be downloaded) in the download process. See https://github.com/PriorLabs/TabPFN/blob/faf092b49b255f0e68270947eeeca4817b0013d4/src/tabpfn/model/loading.py#L143.

What I would do is to remove this line which will default to the default counting rule which is "count on config.json downloads".

Copy link
Contributor

@Wauplin Wauplin Jan 13, 2025

Choose a reason for hiding this comment

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

Actually, this PR has been merged but is not effective on the Hub because models are tagged as TabPFN instead of tabpfn (I'll open PRs to fix this). But if you look at https://huggingface.co/models?other=tabpfn the download count is already there => it defaulted to counting config.json which is correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

opened #1100

packages/tasks/src/model-libraries.ts Outdated Show resolved Hide resolved
packages/tasks/src/model-libraries.ts Outdated Show resolved Hide resolved
@merveenoyan merveenoyan merged commit 74680a1 into main Jan 10, 2025
5 checks passed
@merveenoyan merveenoyan deleted the add-tabpfn branch January 10, 2025 11:20
@merveenoyan
Copy link
Contributor Author

@pcuenca I sent them an email on how to do that in their codebase so the models are automatically downloaded and loaded through Hub when model id is given. Will wait on their reply before doing that.

@noahho
Copy link

noahho commented Jan 10, 2025

Thank you @merveenoyan and @pcuenca for being super responsive 🙏!

@Wauplin
Copy link
Contributor

Wauplin commented Jan 13, 2025

Be careful that tags are case-sensitive so models should be tagged as tabpfn, not TabPFN. I've opened https://huggingface.co/Prior-Labs/TabPFN-v2-clf/discussions/1 and https://huggingface.co/Prior-Labs/TabPFN-v2-reg/discussions/1 so that the connection is correctly made. In the interface it'll still be shown as TabPFN given the "pretty_name" set in this PR (also note that it might take 3-5 days to be shipped to production)

(nit) In general, better to check that https://huggingface.co/models?other=tabpfn is correctly populated before merging this kind of PR.

Wauplin added a commit that referenced this pull request Jan 13, 2025
…on download)

See #1094 (comment).

Better to count only `config.json` downloads to avoid double-counting.

Also, it seems that it should be easy to create a "use this model" snippet for TabPFN models. As mentioned by @merveenoyan in #1094 (comment), documenting it in TabPFN repos + on the Hub will make it easier to load the models for end users. This is orthogonal to this PR though.
@pcuenca
Copy link
Member

pcuenca commented Jan 13, 2025

Thanks @Wauplin, sorry for the hasty merge.

Wauplin added a commit that referenced this pull request Jan 13, 2025
…downloads) (#1100)

See
#1094 (comment).

Better to count only `config.json` downloads to avoid double-counting.

Also, it seems that it should be easy to create a "use this model"
snippet for TabPFN models. As mentioned by @merveenoyan in
#1094 (comment),
documenting it in TabPFN repos + on the Hub will make it easier to load
the models for end users. This is orthogonal to this PR though. cc
@noahho
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