Allow fallback to loading from Auto"SubProcessor".from_pretrained when model_type can't be inferred from config#42402
Conversation
|
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. |
tomaarsen
left a comment
There was a problem hiding this comment.
This PR, when combined with #42387 to fix PEFT loading, fixes my bug reproduction script from #41846. Thanks for opening this @yonigozlan. We should likely aim for a review from a core transformers maintainer, though.
cc @BenjaminBossan this is also looking good for your PR #42387 🤗
|
Thanks, Yoni. Yeah, let's try to get this and my PR merged for v5. |
zucchini-nlp
left a comment
There was a problem hiding this comment.
lgt, only one comment about deleting feature extractors as fallback
| if preprocessor_config_file is not None and processor_class is None: | ||
| config_dict, _ = FeatureExtractionMixin.get_feature_extractor_dict( | ||
| pretrained_model_name_or_path, **kwargs | ||
| ) | ||
| processor_class = config_dict.get("processor_class", None) | ||
| if "AutoProcessor" in config_dict.get("auto_map", {}): | ||
| processor_auto_map = config_dict["auto_map"]["AutoProcessor"] | ||
|
|
There was a problem hiding this comment.
ig this was deleted because feature extractor and image processor have the same name when saved? There is still one difference though if the config was saved in the new nested format. I think we need to keep it
transformers/src/transformers/feature_extraction_utils.py
Lines 486 to 487 in f779506
There was a problem hiding this comment.
I removed this because it is actually never used unless I missed something. It's already called here:
transformers/src/transformers/models/auto/processing_auto.py
Lines 297 to 304 in f779506
and if we
preprocessor_config_file is None above, it will be None here as well. If it's not None, then we'll never enter the code path I removed. So it seems like dead code
There was a problem hiding this comment.
oh you're right! It has a weird dependency because of this identical naming issue 🙃
I think we have to enter this codepath if processor_class is None because we would need to try to load the processor from feature extractor config if we can't still find it at this point. I think we have no tests for this specific case and it's quite rare which is why no issues until today. I still would like to keep it for full functionality
There was a problem hiding this comment.
Hmm I still think this code path is never entered 😅. But this can be addressed in another PR anyway, so I added this back.
|
Thanks for the reviews! @zucchini-nlp could you approve pls ;) |
|
[For maintainers] Suggested jobs to run (before merge) run-slow: auto |
…raise-error-early
…n model_type can't be inferred from config (huggingface#42402) * fix raise error early * add back feature extractor saving logic
…n model_type can't be inferred from config (huggingface#42402) * fix raise error early * add back feature extractor saving logic
What does this PR do?
Fixes #41846
Also remove some dead code in
processing_auto.pyCc @tomaarsen ;)