huggingface / transformers

🤗 Transformers: State-of-the-art Machine Learning for Pytorch, TensorFlow, and JAX.
https://huggingface.co/transformers
Apache License 2.0
135.56k stars 27.14k forks source link

Initializing via AutoImageProcessor before AutoProcessor is imported causes `AttributeError` #34307

Open alex-jw-brooks opened 1 month ago

alex-jw-brooks commented 1 month ago

System Info

Who can help?

Probably @zucchini-nlp @amyeroberts, and @qubvel

Information

Tasks

Reproduction

There seems to be an edge-case in the loading behavior that can sometimes be hit if something is initialized with from_pretrained through AutoImageProcessor before AutoProcessor is imported, and then from_pretrained is used on AutoProcessor.

Repro case - this works as expected

model_name = "microsoft/Phi-3.5-vision-instruct"
from transformers import AutoImageProcessor
from transformers import AutoProcessor
AutoImageProcessor.from_pretrained(model_name, trust_remote_code=True)
AutoProcessor.from_pretrained(model_name, trust_remote_code=True)

But this breaks:

model_name = "microsoft/Phi-3.5-vision-instruct"
from transformers import AutoImageProcessor
AutoImageProcessor.from_pretrained(model_name, trust_remote_code=True)

from transformers import AutoProcessor
AutoProcessor.from_pretrained(model_name, trust_remote_code=True)

with AttributeError: module transformers has no attribute Phi3VImageProcessor.

I've tried to reproduce this with a few other models, but haven't been able to yet. It is also probably worth noting that the AutoProcessor doesn't return the same class as AutoImageProcessor for this model, which might matter

Expected behavior

AutoImageProcessor / AutoProcessor from_pretrained should have the same behavior if possible, regardless of import and invocation order

LysandreJik commented 1 month ago

Seems like a bug indeed, thanks for the great report! cc @Rocketknight1, can you take a look at this?

Rocketknight1 commented 1 month ago

Confirmed the bug, and thanks for the clean reproduction script! I suspect this is an issue with how we construct the autoclass mappings, I'm working on it now.

alex-jw-brooks commented 1 month ago

Awesome, thank you both! 🙂

Rocketknight1 commented 1 month ago

After investigation, the issue seems to be here. Manually setting an attribute of the transformers library like that seems to break other stuff in our imports, and it interacts with the AutoProcessor import in ways I don't fully understand

Rocketknight1 commented 1 month ago

Final diagnosis:

cc @zucchini-nlp - can you see an obvious fix to that method that would work for custom classes? I can take this if you're overloaded!

zucchini-nlp commented 1 month ago

Hey @Rocketknight1, thanks for investigating this.

The quickest worrkaround I see is to change this line in Microsoft repo to be an AutoImageProcessor. Just tried it by modifying the code locally, works as expected because the AutoClass will be importable in any case. I am not sure though that forcing autoclass within our loading code would work in all cases, since some classes can be non-automappable? 🤔

But I agree that we need a better solution here than forcing folks on the hub to use AutoClasses. I cannot say right now what would be the best option, and yeah would be super cool if you have any ideas for long-term solution 😄

Rocketknight1 commented 1 month ago

Yeah, that's a really good point! Let me investigate this when I get a chance, maybe I can come up with a (semi-) clean solution.

github-actions[bot] commented 5 days ago

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

DarkLight1337 commented 5 days ago

Any update on this?