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

Fix default revision for pipelines #33395

Merged
merged 5 commits into from
Sep 12, 2024

Conversation

ankane
Copy link
Contributor

@ankane ankane commented Sep 9, 2024

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

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

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

Copy link
Member

@Rocketknight1 Rocketknight1 left a 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!

@ankane
Copy link
Contributor Author

ankane commented Sep 10, 2024

Thanks @Rocketknight1. I'd prefer to keep this one simple.

@HuggingFaceDocBuilderDev

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.

@Rocketknight1
Copy link
Member

@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 main?

@ankane
Copy link
Contributor Author

ankane commented Sep 11, 2024

Just updated, but still getting a Could not find a usable config.yml CircleCI failure.

@LysandreJik
Copy link
Member

@ankane
Copy link
Contributor Author

ankane commented Sep 12, 2024

Hi @LysandreJik, I'm not sure I have an active CircleCI account. If one is needed to contribute, will close this out.

@LysandreJik
Copy link
Member

I think I can help this pass by just pushing your branch elsewhere; let me do that quickly.

@LysandreJik
Copy link
Member

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:

git remote add ankane https://github.com/ankane/transformers
git fetch ankane
git checkout pipeline-revision
git checkout -b pipeline-revision-mirror
git push -u origin pipeline-revision-mirror

@ankane
Copy link
Contributor Author

ankane commented Sep 12, 2024

Thanks @LysandreJik, pushed a fix for the Ruff failure in the commit above.

@LysandreJik
Copy link
Member

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 transformers 🤗

@LysandreJik
Copy link
Member

I'll let @Rocketknight1 double check and merge if happy.

@Rocketknight1
Copy link
Member

Rocketknight1 commented Sep 12, 2024

Yes, looks good to me, and CI is green now. Thank you for the PR!

@Rocketknight1 Rocketknight1 merged commit d71d6cb into huggingface:main Sep 12, 2024
21 checks passed
@ankane
Copy link
Contributor Author

ankane commented Sep 12, 2024

Thanks

itazap pushed a commit to NielsRogge/transformers that referenced this pull request Sep 20, 2024
* 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>
amyeroberts pushed a commit to amyeroberts/transformers that referenced this pull request Oct 2, 2024
* 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>
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.

Pipelines download the latest revision by default instead of the default/output revision
4 participants