rope_theta is ignored on the dense attention path (hardcoded 10000 / 1000000)
还没有人认领这个 Issue。
评估
- 难度
- 4/5
- 预计耗时
- 3-5 天
- 新手友好度
- 64/100
- Issue 类型
- 缺陷
- 描述清晰度
- 基本清楚
- 活跃度
- 活跃
调研方向
先从 activations.h 中的密集时间尺度构造和 ops/ops.h 中的 CreateInvTimescale 开始,然后检查 configs.h 和 python/configs.cc 中的 ModelConfig 序列化。检查 attention.cc 以了解全局表的选择。完成的标准是:已配置的局部和全局 theta 值经过序列化并得到使用,同时不改变现有输出;请通过现有的 goldens 进行验证。
由索引模型根据 Issue 内容生成。
描述
ModelConfig::rope_theta (configs.h:852) is read in exactly one place — the mla_* timescales in struct Activations (activations.h:512-520), i.e. the MLA path. The dense path builds its two tables from constants instead:
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 — the 10000.0 for the local table comes from the default argument at ops/ops.h:30.)
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
- Pass
config.rope_thetato the localCreateInvTimescalecall (configis already a ctor parameter, so no new plumbing). - Add
ModelConfig::global_rope_theta = 1000000.0f, appended at the end ofVisitFieldsfor serialization compatibility, plus the matching entry inpython/configs.cc; pass it to the global call. - Optionally drop the
base_frequencydefault fromCreateInvTimescaleso every call site has to state its theta. - 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), 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 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.)
- 主要语言
- C++
- 星标
- 7k
- 派生
- 660
- 平均合并
- 1 天 3 小时
- 30 天内合并 PR
- 35
贡献指南
从这里开始
- 先读完整个 Issue,再读项目的贡献指南。
- 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
- Fork 仓库,在一个分支上完成修改。
- 提交 Pull Request,并在描述里引用这个 Issue 编号。
google/gemma.cpp 的其他 Issue
-
难度 2/5 1-3 小时 新手友好度 75/100
-
难度 5/5 一周以上 新手友好度 35/100
-
难度 4/5 3-5 天 新手友好度 68/100
-
难度 5/5 一周以上 新手友好度 35/100
-
难度 5/5 一周以上 新手友好度 35/100
相似的 Issue
-
难度 2/5 1-3 小时 新手友好度 75/100
flutter-webrtc/flutter-webrtc#2206 ·
-
难度 2/5 1-3 小时 新手友好度 70/100
google-ai-edge/LiteRT-LM#3739 ·
-
Component: GLib
难度 2/5 1-3 小时 新手友好度 70/100
-
难度 2/5 1-3 小时 新手友好度 65/100
brave/brave-browser#59300 ·
-
Mute ydb/tests/functional/dstool/test_canonical_requests.py.Test.test_group_take_snapshot in main 未关闭ai_reviewed
难度 2/5 1-3 小时 新手友好度 70/100
ydb-platform/ydb#53974 · 3 条评论 ·