fix(MODEL-TEXT-commandr-cohere-for-causal-lm): a Cohere config carrying rms_norm_eps runs mean-centred LayerNorm silently
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 423
- Forks
- 53
- Avg merge
- 20h 26m
- Merged PRs (30d)
- 310
Description
Row: MODEL-TEXT-commandr-cohere-for-causal-lm (.agents/model-matrix.md:216, BLOCKED)
Found by wave PORTQ-5 (#2679) re-deriving PORT-NOW entry [190], upstream aa9903490c vllm#50156. Nothing was executed.
One of the commit's four halves is portable
Upstream adds an RMSNorm class and select_norm_impl(config), which picks RMSNorm + rms_norm_eps when that key is present and LayerNorm + layer_norm_eps otherwise, applied to q_norm / k_norm / input_layernorm / the final norm.
This tree hard-wires Cohere LayerNorm with no selection and no refusal:
src/vllm/model_executor/models/commandr.cpp:52-57—LayerNormInto(...)->vt::LayerNorm(..., /*bias=*/nullptr, ...), called at:159(input_layernorm) and:209(final norm).- Eps comes from one place:
src/vllm/model_executor/models/commandr_registry.cpp:108-111,RawDouble(config.raw, "layer_norm_eps", 1e-5).
Nothing reads rms_norm_eps (zero hits across the four commandr files; positive control through the identical probe form: layer_norm_eps => commandr_registry.cpp:108,110 and commandr.h:98). So a checkpoint of the new Cohere flavour would run mean-centred LayerNorm at a 1e-5 default in place of RMSNorm — a silent numerics divergence, not a refusal, which is the part that makes this worth filing against a BLOCKED row.
The other three halves are not portable
head_dim: already satisfied, at the shared config layer rather than in the model —src/vllm/transformers_utils/hf_config.cpp:448-452honours an explicithead_dim > 0and derives otherwise, consumed atcommandr.cpp:90,commandr_weights.cpp:60,commandr_registry.cpp:119,143.- The
sliding_window + 1training convention: surface-absent. Sliding window is refused on this arch at parse (commandr_registry.cpp:134-138), matching upstream's ownif not self.v1guard. Thecohere2_moe.pyhalf has no file here; that belongs toMODEL-TEXT-cohere2-moe-cohere2-moe-for-causal-lm(.agents/model-matrix.md:217,INVENTORIED). - The single fused all-reduce of attention + MLP: surface-absent. commandr here is single-device — no TP symbol occurs in either commandr TU or its header, and
TpAllReduceSumexists in exactly one model TU (src/vllm/model_executor/models/qwen3.cpp:148). With no two all-reduces there is no pair to fuse.
Size, and an honest cheaper option
Full fix: an RmsNormInto sibling to LayerNormInto (vt::RmsNorm already exists), a CommandrNormKind / eps resolver beside CommandrLayerNormEps, threaded through commandr.cpp:159,209. The q_norm / k_norm arm is moot because use_qk_norm is refused at parse. ~40-60 lines product + ~40 lines test.
Cheaper and arguably more honest while the row is BLOCKED with no runnable vehicle: refuse a config carrying rms_norm_eps by name, which is this tree's own idiom at commandr_registry.cpp:131-138. ~15 lines. A refusal is strictly better than silently wrong numerics.
Not established
Whether any published Cohere checkpoint actually ships rms_norm_eps. No checkpoint was fetched, and the row is BLOCKED on vehicles, so the gap may be latent rather than live.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Read src/vllm/model_executor/models/commandr.cpp and commandr_registry.cpp, starting at LayerNormInto and the layer_norm_eps parsing and refusal logic. Determine whether the intended scope is RMSNorm support or refusing configs containing rms_norm_eps; done means such configs no longer silently use LayerNorm, with coverage for the chosen behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- machine-learning
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100