[Llama] Support attention_bias and mlp_bias config flags - #3494
[Llama] Support attention_bias and mlp_bias config flags#3494przemek-arch wants to merge 2 commits into
Conversation
HuggingFace Transformers PR #30031 (May 2024) added attention_bias and mlp_bias flags to LlamaConfig for llama-variant models that include bias terms in attention and MLP linear layers (IBM Calico, SpeakLeash Bielik v3, future variants). MLC's stock llama_model.py hardcoded bias=False on the QKV / output / gate-up / down projections, so these models could not be converted; the qwen2 fallback only carries the fused QKV bias and silently dropped 240 of 420 HF bias vectors per Bielik conversion. This change adds optional bias support to the Llama implementation: - LlamaConfig: new attention_bias / mlp_bias fields, default False. - LlamaAttention / LlamaFFN: the four projections honor the new flags. - LlamaDecoderLayer._set_tp: column-parallel biases (qkv, gate_up) are sharded along the same dim as the corresponding weight; row-parallel biases (o_proj, down_proj) remain replicated and are added once after allreduce. - make_standard_hf_loader: new add_gate_up_bias / gate_up_bias_optional parameters mirror the existing add_qkv_bias fusion (concat HF gate_proj.bias + up_proj.bias along gate_up_concat_axis). - Llama loader passes *_bias=True with *_bias_optional=True so biases load when the model has bias=True and are skipped otherwise. Backward compatibility is preserved bit-identically: re-converting unsloth/Llama-3.2-1B-Instruct with q4f16_1 produces 22/22 params_shard binaries identical to the unpatched HEAD output, and tensor-cache.json matches. Forward path verified on speakleash/Bielik-4.5B-v3.0-Instruct (60 layers, attention_bias=true, mlp_bias=true): conversion captures all 420 HF bias vectors (vs 180 with the qwen2 workaround), Metal compile succeeds, and the runtime LlamaConfig log echoes attention_bias=True, mlp_bias=True from config.json. Refs: - HF transformers PR #30031: huggingface/transformers#30031 - speakleash/Bielik-4.5B-v3.0-Instruct (Apache 2.0) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces support for attention and MLP biases in the Llama model and its standard loader. It updates the configuration, model layers, and loading logic to handle these biases, including initial support for sharding them during Tensor Parallelism. However, a significant issue was identified in the Tensor Parallelism implementation: row-parallel biases for o_proj and down_proj are added before the all-reduce operation, which will cause the bias to be multiplied by the number of shards. This needs to be corrected by either scaling the bias or adding it after the reduction.
| def _set_bias(layer, hint): | ||
| # Column-parallel layers shard their bias along the same dim as the weight's | ||
| # output dim. Row-parallel layers (o_proj, down_proj) replicate the bias | ||
| # across shards — it is added once to the post-allreduce sum, so no | ||
| # shard_strategy is required for them. | ||
| layer.bias.attrs["shard_strategy"] = hint |
There was a problem hiding this comment.
The comment stating that row-parallel biases (for o_proj and down_proj) are "added once to the post-allreduce sum" is currently not reflected in the implementation.
In the current code, these biases are part of the nn.Linear layers, which means they are added to the local result on each shard before the op.ccl_allreduce(out, "sum") operation in _apply_residual (line 236). Consequently, the allreduce will sum the bias across all shards, effectively multiplying it by the number of shards (e.g., in a 2-GPU setup, the bias will be doubled).
To support Tensor Parallelism correctly with biases, you should either:
- Divide the row-parallel biases by the number of shards during loading or in
_set_tp. - Set
bias=Falseforo_projanddown_projand manually add the bias parameter after the allreduce in_apply_residual.
Per the gemini-code-assist review on PR mlc-ai#3494: the o_proj and down_proj biases were applied inside their nn.Linear, i.e. before the ccl_allreduce(sum) in LlamaDecoderLayer._apply_residual. Under tensor_parallel_shards > 1 the all-reduce would then sum the (full, replicated) bias across every shard, yielding bias x N instead of bias x 1. Fix uses the existing MLC idiom: wrap the attention and MLP calls in LlamaDecoderLayer.forward with tp.shard_bias(..., tensor_parallel_shards), which divides the row-parallel bias by the shard count so the subsequent all-reduce restores it. This mirrors how gpt_neox / starcoder2 already handle dense / dense_4h_to_h biases. The qkv_proj and gate_up_proj biases are column-parallel and keep their existing ShardSingleDim shard_strategy (correct as-is). tp.shard_bias is also made a no-op when linear.bias is None, so the Llama default path (attention_bias=False, mlp_bias=False) is unaffected under any shard count. Verified: - Llama 3.2 1B (no bias) reconversion is bit-identical to the pre-patch baseline (22/22 params_shard_*.bin, tensor-cache.json). - Bielik 4.5B v3 (attention_bias=true, mlp_bias=true) reconversion still captures all 420 HF bias vectors; Metal compile and a 3-prompt Polish smoke test are unchanged on a single device (TP=1, where the bug does not manifest). Also updates the _set_bias docstring that prompted the review comment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Good catch — you're right, the I went with option 1, using the existing I also made Backward compat re-verified: Llama-3.2-1B reconversion is bit-identical to the pre-patch baseline (22/22 |
Motivation
HuggingFace Transformers added
attention_biasandmlp_biasflags toLlamaConfigin PR #30031 (May 2024) to support llama-variant models that include bias terms in attention and MLP linear layers. Examples in the wild:mlp_bias=Truefor stability and performanceattention_bias=Trueandmlp_bias=Trueas a deliberate architectural choice for the Polish-language fine-tuneMLC LLM's current
llama_model.pyhardcodesbias=Falseon the QKV, output, gate-up, and down projections, so converting bias-laden llama variants fails at weight load. The common workaround (using--model-type qwen2, which natively carries a fused QKV bias) silently drops 4 of 7 bias vectors per layer (o_proj.bias,gate_proj.bias,up_proj.bias,down_proj.bias) — for Bielik 4.5B v3 (60 layers) that is 240 of 420 HF bias vectors lost.This PR adds optional bias support to MLC's Llama implementation, defaulting to
Falsefor full backward compatibility with existing Llama 2/3/3.1/3.2 conversions.Changes
python/mlc_llm/model/llama/llama_model.pyLlamaConfigdataclass — addedattention_bias: bool = Falseandmlp_bias: bool = FalsefieldsLlamaAttention.__init__—qkv_projando_projnow usebias=config.attention_biasLlamaFFN.__init__—gate_up_projanddown_projnow usebias=config.mlp_biasLlamaDecoderLayer._set_tp— added a_set_biashelper that shards column-parallel biases (qkv, gate_up) along the same dim as the corresponding weight. Row-parallel biases (o_proj, down_proj) remain replicated across shards (added once after allreduce).python/mlc_llm/loader/standard_loader.pymake_standard_hf_loader— addedadd_gate_up_bias: bool = Falseandgate_up_bias_optional: bool = Falseparameters. The fusion logic mirrors the existingadd_qkv_biaspath: HFgate_proj.bias+up_proj.biasare concatenated alonggate_up_concat_axisinto MLC's fusedgate_up_proj.bias.python/mlc_llm/model/llama/llama_loader.pyadd_qkv_bias=True, qkv_bias_optional=True, add_gate_up_bias=True, gate_up_bias_optional=True. With*_optional=True, the loader picks up bias parameters when the model was instantiated withbias=True(so the bias keys are present innamed_parameters), and skips them otherwise — backward compatibility with bias-free Llama configs is preserved.The
o_proj.biasanddown_proj.biasparameters are passed through the existing 1:1 generic mapping at the end of the loader (no fusion needed, names match HF directly).Testing
Backward compatibility — bit-identical regression
Converted
unsloth/Llama-3.2-1B-Instruct(a bias-free Llama 3.2 mirror) twice with--quantization q4f16_1:mainHEAD (baseline)Then byte-compared every
params_shard_*.bin:The
attention_bias=False, mlp_bias=Falsedefault path is provably unchanged.Forward path — Bielik 4.5B v3 conversion
Converted
speakleash/Bielik-4.5B-v3.0-Instruct(60 layers,attention_bias=True,mlp_bias=True, 32K context) with--quantization q4f16_1 --model-type llama:Tensor-cache analysis vs the previous qwen2-workaround conversion:
--model-type qwen2(workaround)--model-type llama(this PR)Compile to Metal succeeds; the runtime config log confirms
attention_bias=True, mlp_bias=Truepropagating fromconfig.jsonthrough to TVM compilation:Functional smoke test (3 prompts in Polish, including domain-specific legal Q&A) — all produced coherent, well-structured Polish output with appropriate legal terminology.
Empirical quality note — important caveat
We ran a head-to-head A/B against the qwen2-workaround conversion and an FP16/bfloat16 reference (HF transformers, Apple Metal MPS) on:
Result on multistep reasoning (n=5): the qwen2-workaround conversion tracked the FP16 reference more faithfully than the patched fullbias conversion — qwen2 won 2/5 prompts on FP16-faithfulness, fullbias won 0/5, 3/5 ties. This PR brings the Llama architecture to parity with HF transformers (PR #30031), but on this particular model/quantization the extra biases did not improve — and on multistep reasoning slightly hurt — output quality.
We do not yet have a clean explanation. Plausible hypotheses:
q4f32_1(4-bit weights, fp32 activations) recovered some quality on a single multistep prompt but did not generalize across the n=5 set, so the bottleneck is bias precision in q4 group quant rather than activation precision.So this PR is offered as an architectural correctness contribution (Llama parity with HF transformers), not as a measured quality improvement for Bielik q4f16_1. Downstream users converting bias-laden models should benchmark output quality against alternative conversion paths for their specific model + quantization rather than assuming full bias coverage is strictly better. Use cases most likely to benefit:
q5f16,q8f16, no-quant) where bias precision noise is smaller relative to signalHappy to take reviewer guidance on whether this should land as-is, or whether bias precision in group quantization deserves a follow-up patch first. (No existing MLC issue/PR found for
attention_bias/mlp_biasLlama support at the time of writing — if there is a tracking issue I missed, please point me to it.)References
attention_bias/mlp_biastoLlamaConfig: add mlp bias for llama models huggingface/transformers#30031Side benefit
Enables MLC LLM browser-native deployment of:
Diff summary
All changes default to existing behavior; the only behavioral change is when
config.attention_biasorconfig.mlp_biasareTrue(previously a weight-loading failure, now succeeds with proper bias handling).Acknowledgements