Fix helper fn for new processor config format #42085
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. |
molbap
left a comment
molbap
left a comment
There was a problem hiding this comment.
Thanks, looks fine to me! just a couple questions
| if "audio_processor" in feature_extractor_dict: | ||
| feature_extractor_dict = feature_extractor_dict["audio_processor"] | ||
| else: | ||
| feature_extractor_dict = feature_extractor_dict.get("feature_extractor", feature_extractor_dict) |
There was a problem hiding this comment.
I'm a bit confused by the logic here, can you remind me why we need to get this / why audio_processor is directly accessible?
There was a problem hiding this comment.
This is to get the actual config from nested structure if we're loading from processor_config.json. In audio models we have no standardization unfortunately, and some call the attribute as audio_processor or feature_extractor
We need to get any of the two keys if available, otherwise just return
There was a problem hiding this comment.
Crystal clear, thanks, actual audio processors will help a lot when they arrive, cc @eustlb for reference
There was a problem hiding this comment.
Oh yes, much needed!
| ) | ||
| return {} | ||
|
|
||
| resolved_config_file = resolved_config_files[0] |
There was a problem hiding this comment.
So this means if two are present we don't take the PROCESSOR, why do we iterate on both?
There was a problem hiding this comment.
good point, I haven't noticed that we're prioritizing IMAGE_PROCESSOR here. Imo the priority should be as follows if we find more than one file on the hub:
PROCESSOR -> IMAGE_PROCESSOR and for videos PROCESSOR -> VIDEO_PROCESSOR -> IMAGE_PROCESSOR
Do you think this priority makes sense? I will update accordingly
There was a problem hiding this comment.
Yeah it respects the encapsulation of concerns for this set of class, IMO ok!
|
[For maintainers] Suggested jobs to run (before merge) run-slow: auto |
3 participants
What does this PR do?
As per title, these helpers are used in vLLM and don't work if we save a processor after v5. Now all processor's subcomponents are saved in one single place in
processor_config.json, so we need to check it as well when trying to locate the config filefyi @hmellor