[Pipelines] Add revision tag to all default pipelines by patrickvonplaten · Pull Request #17667 · huggingface/transformers

patrickvonplaten · 2022-06-10T15:43:23Z

What does this PR do?

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.

HuggingFaceDocBuilderDev · 2022-06-10T15:55:04Z

The documentation is not available anymore as the PR was closed or merged.

patrickvonplaten · 2022-06-10T16:02:43Z

@Narsil would this be ok for you? see: #17666

I can add a simple slow test that iterates over the default pipelines to make sure that the pipeline is correctly loaded

Narsil

LGTM

src/transformers/pipelines/__init__.py

LysandreJik

Perfect, LGTM!

src/transformers/pipelines/base.py

Co-authored-by: Julien Chaumond <julien@huggingface.co>

…to revision_tags_for_default_pipeline

…om/patrickvonplaten/transformers into revision_tags_for_default_pipeline

patrickvonplaten · 2022-06-30T12:49:51Z

src/transformers/pipelines/__init__.py

        "default": {
-            "model": {"pt": "facebook/bart-large-mnli", "tf": "roberta-large-mnli"},
-            "config": {"pt": "facebook/bart-large-mnli", "tf": "roberta-large-mnli"},
-            "tokenizer": {"pt": "facebook/bart-large-mnli", "tf": "roberta-large-mnli"},


@Narsil before merging this PR, I'd love to have your opinion here. I've checked a the pipeline function and I don't think a "default" tokenizer is never used. If no repo_id is provided it seems like for tokenizer or feature extractor it's always the model id that is used and never the tokenizer id => to me it looks like this is dead code here. Can you confirm?

patrickvonplaten · 2022-06-30T12:51:34Z

tests/pipelines/test_pipelines_common.py

+
+    @slow
+    @require_torch
+    def test_load_default_pipelines_pt(self):


I test here that the default model that is loaded by calling pipeline(<task_name>) indeed loads the corresponding model. Weights are compared to be sure it's actually exactly the same model.

This test is run for all pipelines and should in general serve as a good test to make sure all default pipelines work as expected

patrickvonplaten · 2022-06-30T12:52:46Z

tests/pipelines/test_pipelines_common.py

+    @slow
+    @require_tf
+    @require_tensorflow_probability
+    def test_load_default_pipelines_tf_table_qa(self):


split the test here into table_qa and no table_qa because of the scatter and tensorflow_prop dependencies which are quite annoying and in case one is not installed I still want to run all the other tasks

patrickvonplaten · 2022-06-30T12:55:44Z

src/transformers/pipelines/__init__.py

        "default": {
            "model": {
-                "pt": "google/tapas-base-finetuned-wtq",
-                "tokenizer": "google/tapas-base-finetuned-wtq",


deleted tokenizer because of https://github.com/huggingface/transformers/pull/17667/files#r910987282

patrickvonplaten · 2022-06-30T12:55:53Z

src/transformers/pipelines/__init__.py

-                "tokenizer": "dandelin/vilt-b32-finetuned-vqa",
-                "feature_extractor": "dandelin/vilt-b32-finetuned-vqa",


deleted tokenizer and feature extractor because of https://github.com/huggingface/transformers/pull/17667/files#r910987282

patrickvonplaten · 2022-06-30T12:56:58Z

PR is good to go for me.

@Narsil could you please take a look at this comment: #17667 (comment) before merging - I think there is some dead code. Default tokenizer never seem to be called.

Also cc @sgugger PR should be ready otherwise

sgugger

Thanks for working on this!

src/transformers/pipelines/__init__.py

sgugger · 2022-06-30T13:00:48Z

tests/pipelines/test_pipelines_common.py

        outputs = [item for item in dataset]
        self.assertEqual(outputs, [[{"id": 2}, {"id": 3}, {"id": 4}, {"id": 5}]])
+
+    def check_models_equal_pt(self, model1, model2):


You are adding all of this in a Tester that is under require_pt. I think you need to make a new Tester class since there are TensorFlow tests too.

Good catch - thanks!

Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>

patrickvonplaten · 2022-06-30T14:37:14Z

@Narsil approved offline (tokenizer_default code is dead indeed) => merging!

) * trigger test failure * upload revision poc * Update src/transformers/pipelines/base.py Co-authored-by: Julien Chaumond <julien@huggingface.co> * up * add test * correct some stuff * Update src/transformers/pipelines/__init__.py Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com> * correct require flag Co-authored-by: Julien Chaumond <julien@huggingface.co> Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>

trigger test failure

d54a4cb

upload revision poc

ca6a820

patrickvonplaten changed the title ~~trigger test failure~~ [Pipelines] Add revision tag to all default pipelines Jun 10, 2022

Narsil approved these changes Jun 11, 2022

View reviewed changes

src/transformers/pipelines/__init__.py Outdated Show resolved Hide resolved

LysandreJik approved these changes Jun 13, 2022

View reviewed changes

julien-c reviewed Jun 13, 2022

View reviewed changes

src/transformers/pipelines/base.py Outdated Show resolved Hide resolved

patrickvonplaten and others added 5 commits June 21, 2022 14:02

Update src/transformers/pipelines/base.py

3623fed

Co-authored-by: Julien Chaumond <julien@huggingface.co>

up

38d686d

add test

ab5324e

Merge branch 'main' of https://github.com/huggingface/transformers in…

618aee5

…to revision_tags_for_default_pipeline

Merge branch 'revision_tags_for_default_pipeline' of https://github.c…

2fcd9b6

…om/patrickvonplaten/transformers into revision_tags_for_default_pipeline

patrickvonplaten commented Jun 30, 2022

View reviewed changes

correct some stuff

24205b8

patrickvonplaten commented Jun 30, 2022

View reviewed changes

patrickvonplaten requested review from Narsil and sgugger June 30, 2022 12:53

patrickvonplaten commented Jun 30, 2022

View reviewed changes

sgugger approved these changes Jun 30, 2022

View reviewed changes

patrickvonplaten and others added 2 commits June 30, 2022 15:08

Update src/transformers/pipelines/__init__.py

4e4ff87

Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>

correct require flag

f8ef027

patrickvonplaten merged commit e4d2588 into huggingface:main Jun 30, 2022

patrickvonplaten deleted the revision_tags_for_default_pipeline branch June 30, 2022 14:37

MarcoGorelli mentioned this pull request Jan 31, 2025

Docs: return type of get_default_model_and_revision might be incorrectly documented? #35981

Closed

		"tokenizer": "dandelin/vilt-b32-finetuned-vqa",
		"feature_extractor": "dandelin/vilt-b32-finetuned-vqa",

Conversation

patrickvonplaten commented Jun 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Before submitting

Who can review?

Uh oh!

HuggingFaceDocBuilderDev commented Jun 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patrickvonplaten commented Jun 10, 2022

Uh oh!

Narsil left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LysandreJik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

patrickvonplaten Jun 30, 2022

Choose a reason for hiding this comment

Uh oh!

patrickvonplaten Jun 30, 2022

Choose a reason for hiding this comment

Uh oh!

patrickvonplaten Jun 30, 2022

Choose a reason for hiding this comment

Uh oh!

patrickvonplaten Jun 30, 2022

Choose a reason for hiding this comment

Uh oh!

patrickvonplaten Jun 30, 2022

Choose a reason for hiding this comment

Uh oh!

patrickvonplaten Jun 30, 2022

Choose a reason for hiding this comment

Uh oh!

patrickvonplaten commented Jun 30, 2022

Uh oh!

sgugger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sgugger Jun 30, 2022

Choose a reason for hiding this comment

Uh oh!

patrickvonplaten Jun 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patrickvonplaten commented Jun 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

patrickvonplaten commented Jun 10, 2022 •

edited

Loading

HuggingFaceDocBuilderDev commented Jun 10, 2022 •

edited

Loading

patrickvonplaten Jun 30, 2022 •

edited

Loading