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_theta to the local CreateInvTimescale call (config is already a ctor parameter, so no new plumbing).
- 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.
- Optionally drop the
base_frequency default from CreateInvTimescale so 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.)
ModelConfig::rope_theta(configs.h:852) is read in exactly one place — themla_*timescales instruct Activations(activations.h:512-520), i.e. the MLA path. The dense path builds its two tables from constants instead:(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_timescaleplus 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_thetalooks 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
config.rope_thetato the localCreateInvTimescalecall (configis already a ctor parameter, so no new plumbing).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.base_frequencydefault fromCreateInvTimescaleso every call site has to state its theta.This is behavior-preserving: the only assignment of
rope_thetaanywhere 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_timescaleis arguably redundant: the selector at attention.cc:156 could use the global table wheneverglobal_rope_theta != rope_theta. That removes a flag you can forget to set — setting onlyglobal_rope_thetatoday 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_factoris 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.)