google / google/gemma.cpp

rope_theta is ignored on the dense attention path (hardcoded 10000 / 1000000)

オープン
#986 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
C++
スター
7k
フォーク
660
平均マージ
20時間 43分
マージ済み PR(30日)
33

説明

`ModelConfig::rope_theta` ([configs.h:852](https://github.com/google/gemma.cpp/blob/dev/gemma/configs.h#L852)) is read in exactly one place — the `mla_*` timescales in `struct Activations` ([activations.h:512-520](https://github.com/google/gemma.cpp/blob/dev/gemma/activations.h#L512-L520)), i.e. the MLA path. The dense path builds its two tables from constants instead:

```cpp
inv_timescale(
CreateInvTimescale(allocator, layer_config.qkv_dim,
layer_config.post_qk == PostQKType::HalfRope)),
inv_timescale_global(
CreateInvTimescale(allocator, max_qkv_dim,
layer_config.post_qk == PostQKType::HalfRope,
1000000.0, config.partial_rotary_factor)) {
```

([activations.h:126-132](https://github.com/google/gemma.cpp/blob/dev/gemma/activations.h#L126-L132) — the 10000.0 for the local table comes from the default argument at [ops/ops.h:30](https://github.com/google/gemma.cpp/blob/dev/ops/ops.h#L30).)

**Nothing is wrong today.** Gemma 2 / PaliGemma / T5Gemma / Gemma3-270M want 10000 on every layer; Gemma 3 and Gemma 4 want 10000 local + 1e6 global; Qwen3 wants 1e6 and reaches it via `use_global_timescale` plus all-global windows. Every model in the tree matches one of the two constants, so this is latent rather than a live bug.

What concerns me is the failure mode for the *next* model. `rope_theta` looks like the knob for this, silently isn't on the dense path, and a mismatch produces no assert and no warning — just quietly degraded output that doesn't look like a position-encoding problem. A Llama-style 500000, or any future Gemma with different values, would run at 10000/1e6 and merely seem "worse than expected."

### Proposed fix

1. Pass `config.rope_theta` to the local `CreateInvTimescale` call (`config` is already a ctor parameter, so no new plumbing).
2. Add `ModelConfig::global_rope_theta = 1000000.0f`, appended at the end of `VisitFields` for serialization compatibility, plus the matching entry in `python/configs.cc`; pass it to the global call.
3. Optionally drop the `base_frequency` default from `CreateInvTimescale` so every call site has to state its theta.
4. Set both fields explicitly in the Qwen3 configs, so their correctness is stated rather than incidental.

This is behavior-preserving: the only assignment of `rope_theta` anywhere is DeepSeek's `= 10000.0f` ([configs.cc:672](https://github.com/google/gemma.cpp/blob/dev/gemma/configs.cc#L672)), which equals the default, and the new field's default equals the current literal — so every config in the tree produces bit-identical timescales. It should be checkable against existing goldens.

### One design question

After this, `use_global_timescale` is arguably redundant: the selector at [attention.cc:156](https://github.com/google/gemma.cpp/blob/dev/gemma/attention.cc#L156) could use the global table whenever `global_rope_theta != rope_theta`. That removes a flag you can forget to set — setting only `global_rope_theta` today silently does nothing — but it touches the 10 configs that set it. Happy to keep the flag or drop it, whichever you prefer.

Glad to send a PR if this looks right.

(Separately, and I can file it on its own if it's worth a look: `partial_rotary_factor` is passed only to the global table, so on Gemma 4 2B — where 4 of every 5 layers are local — the local layers get full rotary. I couldn't tell from the code whether that asymmetry is intentional.)

コントリビューションガイド

コントリビューションガイドを開く

調査の方向性

activations.h の密なタイムスケール構築と ops/ops.h の CreateInvTimescale から始め、次に configs.h と python/configs.cc の ModelConfig のシリアライズを調べます。グローバルテーブルの選択を理解するために attention.cc を確認します。設定されたローカルおよびグローバルの theta 値がシリアライズされ、既存の出力を変更せずに使用されることを確認できれば完了です。既存の goldens と照合して検証してください。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
cpp, python
領域
machine-learning
issue の種類
バグ
難易度
4/5
見積もり時間
3〜5日
活発さ
活発
明瞭さ
おおむね明確
初心者へのやさしさ
64/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。