fix for custom pipeline configuration #29004

not-lain · 2024-02-13T17:43:00Z

What does this PR do?

fixes configuration file not pointing at a remote repo with custom pipeline architecture

Fixes #28907

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?

@Rocketknight1

not-lain · 2024-02-21T16:37:48Z

@Rocketknight1 could you review this one too ?
fixed the tests (i forgor to update my branch :D )

Rocketknight1 · 2024-02-21T16:43:59Z

Sure, I'll try to take a look at this one and the pipeline upload one!

not-lain · 2024-02-28T16:54:29Z

cc @Rocketknight1 any review on this one

HuggingFaceDocBuilderDev · 2024-02-29T13:40:26Z

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

I reviewed this and I'm going to approve it on the basis that I think it fixes some specific issues and doesn't break anything that currently works, but I think we need much more clarity in the docs and our conceptual model of custom pipelines in future - this was quite a hard PR to review because I wasn't really sure what behaviour our library was "supposed" to have in these cases.

See my comment here for more.

Rocketknight1 · 2024-02-29T17:04:25Z

cc @ArthurZucker for core maintainer review

not-lain · 2024-02-29T19:33:00Z

cc @Rocketknight1 @ArthurZucker
I made this video to explain why this is important https://youtu.be/2LPJ_1QuK90

not-lain · 2024-02-29T19:50:06Z

it also explains the fundamentals of the transformers library on how to load the custom architecture from a repo that has the architecture defined on another one, which is a workaround i used before on this .

it's pretty much always depends on the config.json which is why it's essential to fix it

ArthurZucker

My main question is why don't we update the save_pretrained part as well to make sure what we already have the correct module?
We can keep your changes for the previous checkpoints, and make sure new ones have the full path (like it is the case for tokenizer and model code path)
Also let's maybe add a test close to https://github.com/huggingface/transformers/blob/d03de1c1070cee0fb7b9358669abc3f68ef56b75/tests/pipelines/test_pipelines_common.py#L709 to make sure we can load a custom pipeline from not-laion when it was pushed from laion!

ArthurZucker · 2024-03-04T07:44:58Z

src/transformers/feature_extraction_utils.py

-            feature_extractor_dict["auto_map"] = add_model_info_to_auto_map(
-                feature_extractor_dict["auto_map"], pretrained_model_name_or_path
-            )
+        if not is_local:


just curious as to why we only do it if not local whereas before we did it for local and not local? 🤗

what does is_local mean ?

if i call a model using this

path = "./folder1/folder2/folder3" from transformers import AutoModelForImageClassification model = AutoModelForImageClassification.from_pretrained(path, trust_remote_code=True)

if the path exists in my pc then we load from there

what if the is_local is false ?

then in this case we call the model from the hub and add the tag

is the tag added when we are only calling a custom model from huggingface ?

yes, there is no need to add the tag if we are calling the model from the local pc

hope this answers your questions 🤗

not-lain · 2024-03-04T16:22:03Z

@ArthurZucker in this pr the configuration tag is added when we are calling the model not when we are saving it which is why it is funadamentally different from the other pr.

check more about the config.json in this repo and then try loading the model and check the config after calling it for more details

# Load model directly
from transformers import AutoModelForImageClassification
model = AutoModelForImageClassification.from_pretrained("not-lain/29004", trust_remote_code=True)
model.config

i know it's pretty weird but this is how the transformers library is working rn.

as for the tests

could you create a repo in https://huggingface.co/hf-internal-testing and i'll add a pr in there with a custom pipeline
or you can copy my repo under https://huggingface.co/not-lain/29004 and we can use it as a reference

not-lain · 2024-03-05T13:31:34Z

@ArthurZucker
your confusion is on me, i have wrongly mentioned the issue, now after checking the code and experimenting more i now know the configuration is being changed when we are calling the model and not when we are saving/pushing

again apologies for the confusion

EDIT
the issue still stands for cases such as finetuning and others since we will push wrongly annotated config to a new repo meaning the new repo will have a broken pipeline.
I will also try to add dummy tests soon.

ArthurZucker

Let's wait for the test to do another round of review!

…not-lain/transformers into fix-config-for-custom-pipelines

not-lain · 2024-03-06T13:31:35Z

cc @ArthurZucker
this should be ready for review

not-lain · 2024-03-07T00:39:49Z

@ArthurZucker @Rocketknight1
could you review #29172 first, since i need the push_to_hub feature from there added to the main branch to add more coverage to the tests just to be safe.

i'll update the tests on this pull request right after, sending lots of hugs 🤗🤗

ArthurZucker · 2024-03-27T06:37:31Z

Sorry on our late answers here! I was off for a while !

not-lain · 2024-03-29T01:58:56Z

@ArthurZucker hope you had fun ✨✨, anyways I have updated the tests to be more straight forward and to the point, do let me know if you have any reviews on this

not-lain · 2024-04-08T00:40:34Z

@ArthurZucker there's no need to wait for 29172 anymore since in 323b50d I switched to using the model to push to another repo (case finetuned model with custom pipeline), also friendly pinging for a review

not-lain · 2024-04-14T21:53:26Z

Hi @ArthurZucker
Any reviews on this PR?

not-lain · 2024-04-22T13:06:00Z

@ArthurZucker friendly tagging you here

not-lain · 2024-04-29T12:12:12Z

hey @ArthurZucker , hope you're doing well.
can I get a review on this PR ?

ArthurZucker · 2024-05-20T09:34:11Z

Wow I am sorry @not-lain really my bad here

ArthurZucker

This seems to work well, I think overall I just don't like that we have to duplicate the logic everywhere but that's not on you!
Thanks for your continuous contribution to the library and the ecosytem! And sorry for being so late here

not-lain · 2024-05-20T12:33:43Z

@ArthurZucker it's ok, you have been busy with lots of stuff including llama-3 and I know things are hectic on your side, just remember to take a break once in a while.
Also thanks a lot for the review, and I wish you a wonderful day ✨

* fix for custom pipeline configuration * fix for custom pipelines * remove extra exception * added test for custom pipelines extra tag * format with ruff * limit extra tag for first time only * format with ruff * improve tests for custom pipelines

fix for custom pipeline configuration

3a74092

not-lain mentioned this pull request Feb 13, 2024

Fix custom architectures #28983

Closed

not-lain and others added 3 commits February 15, 2024 19:34

fix for custom pipelines

c03357e

remove extra exception

2c3ae80

Merge branch 'huggingface:main' into fix-config-for-custom-pipelines

99ad0e2

not-lain mentioned this pull request Feb 22, 2024

add push_to_hub to pipeline #29172

Merged

9 tasks

Rocketknight1 mentioned this pull request Feb 22, 2024

wrongly annotated configuration when saving a model that has a custom pipeline #28907

Closed

4 tasks

Rocketknight1 approved these changes Feb 29, 2024

View reviewed changes

Rocketknight1 requested a review from ArthurZucker February 29, 2024 17:04

ArthurZucker reviewed Mar 4, 2024

View reviewed changes

ArthurZucker reviewed Mar 6, 2024

View reviewed changes

not-lain and others added 6 commits March 6, 2024 13:35

added test for custom pipelines extra tag

68e0544

Merge branch 'huggingface:main' into fix-config-for-custom-pipelines

1283454

format with ruff

cd49d7d

Merge branch 'fix-config-for-custom-pipelines' of https://github.com/…

78e4690

…not-lain/transformers into fix-config-for-custom-pipelines

limit extra tag for first time only

2a30970

format with ruff

26866d0

improve tests for custom pipelines

323b50d

Merge branch 'huggingface:main' into fix-config-for-custom-pipelines

091630d

ArthurZucker approved these changes May 20, 2024

View reviewed changes

ArthurZucker merged commit c11ac78 into huggingface:main May 20, 2024
20 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix for custom pipeline configuration #29004

fix for custom pipeline configuration #29004

not-lain commented Feb 13, 2024 •

edited

Loading

not-lain commented Feb 21, 2024

Rocketknight1 commented Feb 21, 2024

not-lain commented Feb 28, 2024

HuggingFaceDocBuilderDev commented Feb 29, 2024

Rocketknight1 left a comment

Rocketknight1 commented Feb 29, 2024

not-lain commented Feb 29, 2024

not-lain commented Feb 29, 2024

ArthurZucker left a comment •

edited

Loading

ArthurZucker Mar 4, 2024

not-lain Mar 4, 2024

not-lain commented Mar 4, 2024

not-lain commented Mar 5, 2024 •

edited

Loading

ArthurZucker left a comment

not-lain commented Mar 6, 2024

not-lain commented Mar 7, 2024

ArthurZucker commented Mar 27, 2024

not-lain commented Mar 29, 2024

not-lain commented Apr 8, 2024

not-lain commented Apr 14, 2024

not-lain commented Apr 22, 2024

not-lain commented Apr 29, 2024

ArthurZucker commented May 20, 2024

ArthurZucker left a comment

not-lain commented May 20, 2024

fix for custom pipeline configuration #29004

fix for custom pipeline configuration #29004

Conversation

not-lain commented Feb 13, 2024 • edited Loading

What does this PR do?

Before submitting

Who can review?

not-lain commented Feb 21, 2024

Rocketknight1 commented Feb 21, 2024

not-lain commented Feb 28, 2024

HuggingFaceDocBuilderDev commented Feb 29, 2024

Rocketknight1 left a comment

Choose a reason for hiding this comment

Rocketknight1 commented Feb 29, 2024

not-lain commented Feb 29, 2024

not-lain commented Feb 29, 2024

ArthurZucker left a comment • edited Loading

Choose a reason for hiding this comment

ArthurZucker Mar 4, 2024

Choose a reason for hiding this comment

not-lain Mar 4, 2024

Choose a reason for hiding this comment

not-lain commented Mar 4, 2024

not-lain commented Mar 5, 2024 • edited Loading

ArthurZucker left a comment

Choose a reason for hiding this comment

not-lain commented Mar 6, 2024

not-lain commented Mar 7, 2024

ArthurZucker commented Mar 27, 2024

not-lain commented Mar 29, 2024

not-lain commented Apr 8, 2024

not-lain commented Apr 14, 2024

not-lain commented Apr 22, 2024

not-lain commented Apr 29, 2024

ArthurZucker commented May 20, 2024

ArthurZucker left a comment

Choose a reason for hiding this comment

not-lain commented May 20, 2024

not-lain commented Feb 13, 2024 •

edited

Loading

ArthurZucker left a comment •

edited

Loading

not-lain commented Mar 5, 2024 •

edited

Loading