-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
Fix default revision for pipelines #33395
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.
This looks good! (aside from the CircleCI failure, but that's not your fault)
@ankane I'm happy to merge this as-is, but while you're fixing up the default models in the pipelines, would you be willing to help out with a slightly larger project? We were discussing the outdated default models in the pipelines internally, and one project that's been on the list for a while has been to go through them and see if there are any improved defaults.
For example, for a task like visual question answering, the current vilt-b32-finetuned-vqa
could probably be replaced with one of the recent 2B Qwen-VLM
models, or distilbert
could be replaced with deberta
. If you wanted to do some of that work in this PR, I think it could be really useful to the community, but don't stress if you'd prefer to keep this PR simple and not take on the bigger job!
Thanks @Rocketknight1. I'd prefer to keep this one simple. |
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
@ankane everything looks good in that case! There's one failing test which would be solved by a rebase, and then I can merge. Can you rebase your PR branch onto the latest commit to |
0dc88c7
to
764ecfc
Compare
Just updated, but still getting a |
Hey @ankane, thanks for your PR! Could you try following the steps highlighted here to see if it fixes it? https://support.circleci.com/hc/en-us/articles/12504745606171-Why-am-I-seeing-the-Could-not-find-a-usable-config-yml-you-may-have-revoked-the-CircleCI-OAuth-app-message |
Hi @LysandreJik, I'm not sure I have an active CircleCI account. If one is needed to contribute, will close this out. |
I think I can help this pass by just pushing your branch elsewhere; let me do that quickly. |
I pushed it here manually https://github.com/huggingface/transformers/compare/pipeline-revision-mirror?expand=1 and it launched the tests. Sorry this happens here, it's CircleCI being annoying. @Rocketknight1 in case this happens elsewhere, this is what I did here:
|
764ecfc
to
033ce8c
Compare
Thanks @LysandreJik, pushed a fix for the Ruff failure in the commit above. |
Thanks @ankane! The failing processor test is unrelated to your PR. The PR is good as-is, thanks for your work, and thanks for contributing to |
I'll let @Rocketknight1 double check and merge if happy. |
Yes, looks good to me, and CI is green now. Thank you for the PR! |
Thanks |
* Fix default revision for pipelines * dummy change to trigger CI * revert dummy change * dummy change to trigger CI * revery dummy change --------- Co-authored-by: Matt <rocketknight1@gmail.com>
* Fix default revision for pipelines * dummy change to trigger CI * revert dummy change * dummy change to trigger CI * revery dummy change --------- Co-authored-by: Matt <rocketknight1@gmail.com>
What does this PR do?
Fixes #33365
I've updated the default models to use the latest revision since that's been the behavior of every released version (see the issue above for details).
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.
@LysandreJik @Narsil