Fix tie_word_embedding issues with Qwen2VL#44976
Conversation
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
|
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. |
ArthurZucker
left a comment
ArthurZucker
left a comment
There was a problem hiding this comment.
I am probably missing some context (and previous code already looks super weird).
Might be worse adding a small test as well!
| bos_token_id: int | None = 151643 | ||
| eos_token_id: int | list[int] | None = 151645 | ||
| pad_token_id: int | None = None | ||
| tie_word_embeddings: bool = False |
There was a problem hiding this comment.
its false by default so that does not change much
| if tie_word_embeddings: | ||
| self.vlm_config["tie_word_embeddings"] = tie_word_embeddings.pop() |
There was a problem hiding this comment.
we are checking tie word embedding from both text and vision config to know if we should tie for the vlm?
There was a problem hiding this comment.
I guess the vision config is a VLM as well?
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
|
[For maintainers] Suggested jobs to run (before merge) run-slow: colmodernvbert, colqwen2, modernvbert, qwen2_5_vl, qwen2_vl |
ArthurZucker
left a comment
ArthurZucker
left a comment
There was a problem hiding this comment.
Test can come later, TLDR: vllm reads the attribute. Having it here as false is an error!
* Fix tie_word_embedding issues with `Qwen2VL` Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com> * remove colqwen hack Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com> --------- Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
3 participants
ModernVBertForMaskedLM.__init__tie_word_embeddingsfromQwen2VLTextConfig(and therefore alsoQwen2_5_VLTextConfig) because it's not valid for these modelsColQwen2Config(and therefore alsoColModernVBertConfig) and fix the config upstream instead