huggingface / huggingface/candle

phi3: long-prompt output degrades into word-salad — BF16 RoPE table + ignored `sliding_window` (rope dtype also affects qwen2/gemma/gemma2)

Open
#3,698 3 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
21k
Forks
1.8k
Avg merge
16h 42m
Merged PRs (30d)
25

Description

## Summary

`candle_transformers::models::phi3::Model` (Phi-3-mini-4k-instruct, full-precision SafeTensors) produces coherent output on short prompts but degrades into fluent word-salad as the prompt grows — well below the model's 4096-token window. We isolated this to **two independent bugs**, both still appear to be present in the latest release, 0.11.0 (and on master, checked at 31f35b1):

1. **The RoPE phase table is built in the model dtype.** In BF16 this corrupts position encoding at higher positions. Also affects `qwen2.rs`, `gemma.rs`, and `gemma2.rs`; `mistral.rs` and `llama.rs` already compute the table in F32, which is the fix.
2. **`config.json`'s `"sliding_window": 2047` is ignored.** Phi-3-mini was trained with a 2047-token sliding attention window and HF transformers applies it in the causal mask; candle's phi3 `Config` doesn't have the field and builds a plain causal mask. Past ~2047 prompt tokens the output collapses instantly, on every device and dtype.

## Bug 1 — RoPE table in model dtype (phi3, qwen2, gemma, gemma2)

`RotaryEmbedding::new` builds `t = arange(0, max_seq_len)` and `freqs = t @ inv_freq` in the dtype passed by `Model::new(vb.dtype(), …)`:

https://github.com/huggingface/candle/blob/0.11.0/candle-transformers/src/models/phi3.rs#L90-L93 (and the equivalent in qwen2.rs#L56-L59 / gemma.rs#L92-L95 / gemma2.rs)

With BF16's 8-bit significand:
- positions above 256 no longer round-trip exactly (near position 1600 they quantize to multiples of 8, so runs of 8 tokens share one position id), and
- the phase values — up to thousands of radians — carry ~0.4% relative error, i.e. multiple radians for the high-frequency channels, so `sin`/`cos` of those phases are effectively noise.

The error grows with position, which matches the observed graded degradation exactly (greedy decoding, Phi-3-mini-4k, Metal BF16): perfect at 10 prompt tokens, slight tail drift at ~590, garbled by ~1600.

The fix is the one `mistral.rs` and `llama.rs` already use: compute the table in F32 and cast `cos`/`sin` (values in [-1, 1], where BF16 rounding is harmless) to the model dtype after `sin()`/`cos()`:

https://github.com/huggingface/candle/blob/0.11.0/candle-transformers/src/models/mistral.rs#L127-L135

We verified that applying this one change to phi3 makes a 1616-token prompt decode coherently on Metal BF16 where stock produces garbage. The same change fixes observable long-prompt degradation for `Qwen2.5-0.5B-Instruct` (dropped words, broken grammar at ~2600 tokens, BF16 Metal, greedy) and `gemma-2b-it` (repetition loops at ~2600 tokens). `gemma2.rs` has the same table construction; we didn't test it in BF16 since we run Gemma2 in F32 for the unrelated softcapping-precision reason.

F32/CPU runs are unaffected, which is why the bug is easy to miss — several candle examples default to F32.

## Bug 2 — phi3 ignores `sliding_window` (device/dtype-independent)

`Phi-3-mini-4k-instruct/config.json` sets `"sliding_window": 2047`. HF transformers applies it when building the causal mask, so a query never attends keys more than 2047 positions back. Candle's phi3:

- `Config` has no `sliding_window` field, and
- `prepare_decoder_attention_mask` builds a plain causal mask: https://github.com/huggingface/candle/blob/0.11.0/candle-transformers/src/models/phi3.rs#L386-L404

Past ~2047 prompt tokens, full attention is out-of-distribution for the model and output collapses instantly and completely — identically on CPU F32 and Metal BF16. Example greedy output at 2610 prompt tokens (rope already fixed, so this is isolated):

> Python. Python. Python. Python. Python. Python. Python, the Python. Python this. …

Notably, candle's `mistral.rs`, `qwen2.rs`, and `gemma2.rs` all apply their configured sliding windows in the mask — phi3 is the outlier.

The fix that worked for us (matching the HF convention: mask keys `j` where `query_pos - j >= sliding_window`):

- add `sliding_window: Option` to `Config`;
- apply the window in `prepare_decoder_attention_mask`;
- also mask **single-token decode steps** once `kv_len > sliding_window` — the current `forward` passes `None` for `seq_len <= 1`, so even a correct prefill mask isn't enough when generation continues past the window. (This last point applies to the sibling models too: they only mask when `seq_len > 1`.)

## Evidence matrix (greedy decoding, real Phi-3-mini-4k weights, plain-text prompts)

| Config | Device/dtype | Prompt tokens | Output |
|---|---|---|---|
| stock | Metal BF16 | 1616 | degraded (garbled structure) |
| stock | CPU F32 | 1616 | coherent |
| rope-F32 only | Metal BF16 | 1616 | **coherent** → bug 1 |
| rope-F32 only | Metal BF16 | 2610 | word-salad |
| stock | CPU F32 | 2610 | identical word-salad → bug 2, device-independent |
| both fixes | Metal BF16 | 2610 / 3604 | **coherent** |
| both fixes | Metal BF16 | 2042 prompt + 150 generated | **coherent** across the 2047 decode boundary |

Ruled out along the way: sampling/repetition penalty (greedy fails), NaN/Inf logits (finite and peaked), tokenization/special tokens, exceeding the 4096 window, and prompt content (plain-text padding reproduces it).

## Reproduction

Load `microsoft/Phi-3-mini-4k-instruct` (SafeTensors, BF16 on Metal), build any well-formed chat prompt padded to ≥1600 tokens (bug 1, Metal) or ≥2100 tokens (bug 2, any device), and greedy-decode ~50 tokens with `model.forward(&input, 0)` followed by the usual one-token steps. We used a ~100-line standalone harness and can share it, or open a PR with both fixes if that's welcome.

Environment: observed on candle 0.8.3; both bugs confirmed still present in 0.11.0 (latest release) and on master at 31f35b1 by source inspection. macOS (M3 Max) Metal + CPU, Rust 1.8x.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in candle-transformers/src/models/phi3.rs at RotaryEmbedding::new, Config, prepare_decoder_attention_mask, and forward; compare the existing F32 RoPE handling in mistral.rs and llama.rs and sliding-window handling in qwen2.rs and gemma2.rs. Verify both fixes with long-prompt greedy decoding on CPU F32 and Metal BF16, including generation across the 2047-token boundary, and check the equivalent RoPE code in qwen2.rs, gemma.rs, and gemma2.rs.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
machine-learning
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.