Comfy-Org / Comfy-Org/ComfyUI

Qwen25_7BVLI_Config.lm_head defaults to False but Qwen2.5-VL has untied embeddings — should it be True?

Open
#15,047 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
133k
Forks
15.7k
Avg merge
1d 7h
Merged PRs (30d)
158

Description

## Question

`Qwen25_7BVLI_Config` (`comfy/text_encoders/llama.py`) sets `lm_head: bool = False` with no comment:

```python
@dataclass
class Qwen25_7BVLI_Config(LLAMAConfig):
...
lm_head: bool = False
```

But Qwen2.5-VL-7B does **not** tie word embeddings to the output projection — it ships a separate `lm_head` / `output.weight`. With `lm_head=False`, `Llama2_` does not build a real `lm_head`, so any `transformer.generate()` call falls back to using `embed_tokens` as the output projection, producing wrong / degenerate text for exactly this model.

By contrast, the newer VL configs default to `True`:
- `Qwen3VL_8BConfig` (`llama.py`) — `lm_head: bool = True`
- `qwen35_9b` / `qwen35_27b` (`qwen35.py`) — `lm_head: bool = True`

Those are untied too, so the `False` default on `Qwen25_7BVLI_Config` looks inconsistent rather than intentional.

## Why I'm asking instead of PR-ing

Setting `lm_head=True` loads an extra ~300 MB (quantized) `output.weight` for **every** Qwen2.5-VL load, even pure text-encoder / encode-only usage that never calls `generate()`. That VRAM cost is the only reason I haven't just flipped it.

## What I'd like to know

1. Is `lm_head=False` here intentional (encode-only is the only supported path), or an oversight?
2. If it should be `True`, would maintainers prefer:
- default `True` (simplest, but costs VRAM for all loads), or
- an opt-in flag / env var (e.g. `QWEN25VL_LOAD_LM_HEAD`) defaulting to `False`, so generate-capable users opt in?

Happy to turn the agreed direction into a PR.

Contributor guide

Open the contributing guide

Research direction

Read comfy/text_encoders/llama.py, especially Qwen25_7BVLI_Config and Llama2_, then compare the lm_head settings in Qwen3VL_8BConfig and qwen35.py. Verify whether transformer.generate() uses embed_tokens when lm_head is false, and assess the encode-only VRAM impact versus generation correctness. Done means a maintainer-approved default or opt-in behavior with coverage for both loading paths.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
machine-learning
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.