-
Notifications
You must be signed in to change notification settings - Fork 274
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 image-feature-extraction task to transformers and timm lib->task mapping #1120
base: main
Are you sure you want to change the base?
Conversation
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.
The intent here is to enable the pipeline snippet in the "Use this model" button, and I think it'll work for that purpose. However, I'm not sure whether this will attempt to show an inference widget as well, along with the curl / js snippets. What do you think @SBrandeis?
The image-feature-extraction task is supported for inference here, but not here. The following curl
command hence fails with {"error":"image-feature-extraction is not a valid pipeline"}
curl https://api-inference.huggingface.co/models/facebook/dinov2-large \
-X POST \
--data-binary '@example.jpg' \
-H "Authorization: Bearer $TOKEN"
Also cc @merveenoyan @NielsRogge
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.
for snippets we'd need to add the task here as well AFAIR: https://github.com/huggingface/huggingface.js/tree/44e9e02cee0a3cb9d6e89b329f8c2d60b458e480/packages/tasks/src/snippets
That's for inference snippets / widgets IIUC. For "Use this model", would the current state of the PR work with no impact elsewhere? (i.e., no widget enabled) |
"Use this model" snippets are defined in https://github.com/huggingface/huggingface.js/blob/44e9e02cee0a3cb9d6e89b329f8c2d60b458e480/packages/tasks/src/model-libraries.ts AFAIK |
Yes! The issue is that i.e, this is an vs an My concern is whether updating |
worst case we merge and we'll check in prod:) |
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.
I think this PR should work. I'm a bit late to party, sorry.
I think adding model-libraries.ts will not matter because it's mostly for third party libraries and already defined for transformers. There are two issues here:
- transformers
image-feature-extraction
models don't have pipeline snippets. transformers models are maintained separately AFAIK
them havingpipeline_tag
asimage-feature-extraction
won't matter either e.g. this one has the tag and also have the model class properly but doesn't have snippet mapped (this PR should solve that).
I have overlooked it, thanks for fixing. I will add other vision tasks in a separate PR.
- timm models likewise doesn't have timmwrapper to load with transformers, which I'm surprised because they're inferred from here and
TimmWrapper
exists there. @rwightman opened another PR for it it seems
enabled the same for VLMs with recently merged |
There's no pipeline snippet for models w/ the image-feature-extraction task, probably because this mapping is missing?
cc @pcuenca re slack comment