Skip to content

[vlm] fix loading of retrieval VLMs#39242

Merged
zucchini-nlp merged 11 commits intohuggingface:mainfrom
zucchini-nlp:fix-rerieval-vlm
Jul 15, 2025
Merged

[vlm] fix loading of retrieval VLMs#39242
zucchini-nlp merged 11 commits intohuggingface:mainfrom
zucchini-nlp:fix-rerieval-vlm

Conversation

@zucchini-nlp
Copy link
Copy Markdown
Member

What does this PR do?

As per title, reported internally that slow tests are failing. We need to apply same changes as in VLMs to the models that use VLMs in their architecture

@zucchini-nlp
Copy link
Copy Markdown
Member Author

run-slow: colpali, colqwen2

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 7, 2025

This comment contains run-slow, running the specified jobs:

models: ['models/colpali', 'models/colqwen2']
quantizations: [] ...

@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

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.

@zucchini-nlp zucchini-nlp requested review from Cyrilvallez and ydshieh and removed request for ydshieh July 7, 2025 08:38
@zucchini-nlp
Copy link
Copy Markdown
Member Author

I wanted to use AutoModel as we shouldn't be loading the lm-head for these model. But the Qwen2-based model was released after refactor, and can work without any conversion_key_mapping currently, and I don't want us to add another key mapping just to use AutoModel instead of AutoModelForImageTextToText

Copy link
Copy Markdown
Collaborator

@ydshieh ydshieh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable fix to me (as they seems to apply the same changes made to VLM)

@zucchini-nlp I observed that the 2 model tests fails in 2 different PRs, but maybe they share the same cause so their fix seems identical here?

For context:

colpali tests are failing after

[VLM] Add base model without head (#37033)

And for colqwen2, it fails after

[qwen] refactor attentions for vision/audio (#38930)

and there is a fix [qwen2-vl] fix vision attention scaling #39043, but that one doesn't fix for colqwen

@zucchini-nlp
Copy link
Copy Markdown
Member Author

Hmm, ColQwen for me wasn't failing in a sense that the weights matched when laoding. But the tensors aren't close enough even after model was released. I can check out on runners and see what's the issue.

ColQwen shouldn't have the same issue, it was released after the major refactor

@ydshieh
Copy link
Copy Markdown
Collaborator

ydshieh commented Jul 8, 2025

Hi, sorry, I think my memory got messed

#39043 (comment)

So that issue was already fixed, but my brain wasn't yet.

Comment thread src/transformers/modeling_utils.py Outdated
Copy link
Copy Markdown
Member

@Cyrilvallez Cyrilvallez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding general logic only for colpali, we should be able to add the correct names to tied_weight_keys directly no?

Comment thread src/transformers/modeling_utils.py Outdated
@github-actions
Copy link
Copy Markdown
Contributor

[For maintainers] Suggested jobs to run (before merge)

run-slow: colpali, colqwen2

@zucchini-nlp zucchini-nlp merged commit 9f41f67 into huggingface:main Jul 15, 2025
25 checks passed
rjgleaton pushed a commit to rjgleaton/transformers that referenced this pull request Jul 17, 2025
* fix vlm with retrieval

* we can't use AutoModel because new ColQwen was released after refactor

* no need for colqwen

* tied weight keys are necessary, if using IMageTextToText

* need to apply renaming in tied weights, only for ColPali

* overwrite tied keys in ColPali

* fix copies, modular can't handle if-statements
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
* fix vlm with retrieval

* we can't use AutoModel because new ColQwen was released after refactor

* no need for colqwen

* tied weight keys are necessary, if using IMageTextToText

* need to apply renaming in tied weights, only for ColPali

* overwrite tied keys in ColPali

* fix copies, modular can't handle if-statements
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
* fix vlm with retrieval

* we can't use AutoModel because new ColQwen was released after refactor

* no need for colqwen

* tied weight keys are necessary, if using IMageTextToText

* need to apply renaming in tied weights, only for ColPali

* overwrite tied keys in ColPali

* fix copies, modular can't handle if-statements
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
* fix vlm with retrieval

* we can't use AutoModel because new ColQwen was released after refactor

* no need for colqwen

* tied weight keys are necessary, if using IMageTextToText

* need to apply renaming in tied weights, only for ColPali

* overwrite tied keys in ColPali

* fix copies, modular can't handle if-statements
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
* fix vlm with retrieval

* we can't use AutoModel because new ColQwen was released after refactor

* no need for colqwen

* tied weight keys are necessary, if using IMageTextToText

* need to apply renaming in tied weights, only for ColPali

* overwrite tied keys in ColPali

* fix copies, modular can't handle if-statements
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
* fix vlm with retrieval

* we can't use AutoModel because new ColQwen was released after refactor

* no need for colqwen

* tied weight keys are necessary, if using IMageTextToText

* need to apply renaming in tied weights, only for ColPali

* overwrite tied keys in ColPali

* fix copies, modular can't handle if-statements
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
* fix vlm with retrieval

* we can't use AutoModel because new ColQwen was released after refactor

* no need for colqwen

* tied weight keys are necessary, if using IMageTextToText

* need to apply renaming in tied weights, only for ColPali

* overwrite tied keys in ColPali

* fix copies, modular can't handle if-statements
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
* fix vlm with retrieval

* we can't use AutoModel because new ColQwen was released after refactor

* no need for colqwen

* tied weight keys are necessary, if using IMageTextToText

* need to apply renaming in tied weights, only for ColPali

* overwrite tied keys in ColPali

* fix copies, modular can't handle if-statements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants