Fix Falcon ALiBi bias when using the KV cache#2862
Conversation
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where the ALiBi bias in the Falcon model did not span the full KV-cache length during cached decoding. The FalconTransformerDecoder class in falcon_transformer_decoder.py has been updated to build the ALiBi mask using the full key/value length from the attention_cache when available. A regression test, test_cached_decoding_logits_match_full_forward, has been added to falcon_causal_lm_test.py to verify that cached decoding logits match those of a full forward pass. No review comments were provided, and the implementation looks robust and correct.
The ALiBi bias was built over the current query length while attention scores span the KV-cache length. With cache length > query length this either crashes (broadcast mismatch, e.g. prefilling a longer cache) or silently adds a zero bias during single-token decode steps ([b, n, 1, 1] broadcasts over the cache), corrupting generate() output. Build the bias over the full cache length when a cache is present; positions are absolute and unused cache slots are masked out by the causal mask. The no-cache path is unchanged and produces bit-identical outputs. Fixes keras-team#2861
574546f to
6789a32
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the Falcon transformer decoder to correctly handle ALiBi bias when using an attention cache. Specifically, when attention_cache is provided, the mask used for building the ALiBi tensor is sized to the full cache length (kv_length) rather than the input sequence length. Additionally, a unit test test_cached_decoding_logits_match_full_forward has been added to falcon_causal_lm_test.py to verify that cached decoding logits match those of a full forward pass. I have no feedback to provide as there are no review comments.
|
have you checked the numerics after your edits? |
Summary
Fixes #2861.
Falcon's ALiBi bias was built over the current query length, but the attention scores span the KV-cache length. When the two differ, this breaks in two ways:
call_with_cachewith query length < cache length) raises a broadcast error:The size of tensor a (32) must match the size of tensor b (8) at non-singleton dimension 3.generate(). At every single-token decode step the bias tensor has shape[batch, heads, 1, 1], which broadcasts over the cache length without error — but its value isslope * 0 = 0. Sogenerate()runs to completion while adding no ALiBi bias at all at decode time, and the generated tokens diverge from the correct non-cached output.Fix
In
FalconTransformerDecoder.call, when a KV cache is present, build the ALiBi bias over the full cache length instead of the query length. ALiBi positions are absolute (positionjgets biasslope * j), so an all-ones mask over the cache yields exactly the same per-position values as a full forward pass; unused cache slots are excluded by the causal attention mask. The no-cache code path is untouched.Numeric verification (before → after)
Setup: torch backend, CPU, float32. "Full-forward" = a normal forward pass with no cache (training/inference path).
0.0)falcon_refinedweb_1b_enarchitecture (load_weights=False)0.0)3.3e-074.8e-07generate()vs non-cached greedy referenceSo: the only behavior that changes is the cached path, which was previously broken (crash or wrong outputs); everything that worked before produces bit-identical outputs.
The regression test added in
falcon_causal_lm_test.py(test_cached_decoding_logits_match_full_forward) fails on master with the broadcast error and passes with this fix. Full falcon test suite: 19 passed, 4 skipped.