mudler / mudler/vllm.cpp

fix(MODEL-TEXT-commandr-cohere-for-causal-lm): a Cohere config carrying rms_norm_eps runs mean-centred LayerNorm silently

Open
#2,698 0 comments 0 reactions 0 assignees View on GitHub

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-57LayerNormInto(...) -> 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-452 honours an explicit head_dim > 0 and derives otherwise, consumed at commandr.cpp:90, commandr_weights.cpp:60, commandr_registry.cpp:119,143.
  • The sliding_window + 1 training convention: surface-absent. Sliding window is refused on this arch at parse (commandr_registry.cpp:134-138), matching upstream's own if not self.v1 guard. The cohere2_moe.py half has no file here; that belongs to MODEL-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 TpAllReduceSum exists 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.