Fix conversion for clip models#46406
Conversation
|
run-slow: clip, clipseg, altclip, siglip, siglip2, metaclip_2, chinese_clip, x_clip, llava, llava_next, llava_next_video, paligemma, gemma3, aya_vision, got_ocr2, internvl, vipllava, mistral3, pp_chart2table, video_llava, qwen2_vl, colqwen2, colpali |
|
This comment contains models: ["models/altclip", "models/aya_vision", "models/chinese_clip", "models/clip", "models/clipseg", "models/colpali", "models/colqwen2", "models/gemma3", "models/got_ocr2", "models/internvl", "models/llava", "models/llava_next", "models/llava_next_video", "models/metaclip_2", "models/mistral3", "models/paligemma", "models/pp_chart2table", "models/qwen2_vl", "models/siglip", "models/siglip2", "models/video_llava", "models/vipllava", "models/x_clip"] |
|
run-slow: clip, clipseg, altclip, siglip, siglip2, metaclip_2, chinese_clip, x_clip, llava, llava_next, llava_next_video, paligemma, gemma3, aya_vision, got_ocr2, internvl, vipllava, mistral3, pp_chart2table, video_llava, qwen2_vl, colqwen2, colpali |
|
This comment contains models: ["models/altclip", "models/aya_vision", "models/chinese_clip", "models/clip", "models/clipseg", "models/colpali", "models/colqwen2", "models/gemma3", "models/got_ocr2", "models/internvl", "models/llava", "models/llava_next", "models/llava_next_video", "models/metaclip_2", "models/mistral3", "models/paligemma", "models/pp_chart2table", "models/qwen2_vl", "models/siglip", "models/siglip2", "models/video_llava", "models/vipllava", "models/x_clip"] |
ArthurZucker
left a comment
There was a problem hiding this comment.
can we add a test please of what was broken? 😢
|
The whole |
|
run-slow: sam3 |
|
Workflow Run ⚙️💔 This comment contains |
|
run-slow: sam3 |
|
Workflow Run ⚙️💔 This comment contains |
|
This comment contains models: ["models/sam3"] |
|
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. |
|
Integration tests are all green now in workflow (and failing in today's nightly CI): |
vasqu
left a comment
There was a problem hiding this comment.
I know run-slow didnt say any new failures for the affected models but I think we need to manually check the failures 😢 just in case we missed them similarly to sam. seeing this whole dependency tree makes me think we should be extra careful
This is a careful approval because we need to fix and it seems to work (first glance).
|
Future note: we might need a dep-tree of conversion mapping so easily get possible affected model lists. This is getting out-of-hand as we add more models to it |
|
CI Dashboard: View test results in Grafana |
* fix it * oops, unrelated diff * wtf * just how?
What does this PR do?
Fixes #46402
Should be mapping by class name because the model-type is shared by other PreTrainedModels that do not need any conversion. In case of Sam3, it loads a
ClipTextModelWithProjectionthat in the past manually unwrappedtext_model.text_modelSee:
transformers/src/transformers/models/clip/modeling_clip.py
Lines 951 to 957 in 08810b1