google / google/gemma.cpp

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

Abierto
#986 1 comentario 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
C++
Estrellas
7k
Forks
660
Merge medio
20 h 43 min
PR fusionados (30 d)
33

Descripción

`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.)

Guía de contribución

Abrir la guía de contribución

Línea de trabajo

Comienza con la construcción de la escala temporal densa en activations.h y CreateInvTimescale en ops/ops.h; después, inspecciona la serialización de ModelConfig en configs.h y python/configs.cc. Revisa attention.cc para entender la selección de la tabla global. Se considera terminado cuando los valores theta locales y globales configurados se serialicen y utilicen sin cambiar las salidas existentes; verifícalo con los goldens existentes.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
cpp, python
Área
machine-learning
Tipo de issue
Error
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Activo
Claridad
Bastante claro
Aptitud para principiantes
64/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.