granitemoehybrid rotates only when it says so — position_embedding_type is the switch, not a restatement - #373
Merged
Merged
Conversation
…_embedding_type is the switch, not a restatement
The census recorded this key as a "checked default" that "restates the
scheme already in use". Checking it against the reference instead of
against its name says the opposite. `GraniteMoeHybridConfig` documents
position_embedding_type (`str`, *optional*):
Positional embedding type to be used; defaults to None.
Allowed options: `[None, "rope"]`
and `modeling_granitemoehybrid.py` builds
self.rotary_emb = GraniteMoeHybridRotaryEmbedding(config)
if config.position_embedding_type == "rope" else None
So the default is NO POSITIONAL ENCODING and `"rope"` is the opt-in that
turns rotation on. `rope_theta` is declared either way — granite-4.0-micro
ships 10000000 — so reading the theta and rotating is right about that
checkpoint by luck and wrong about any granitemoehybrid that omits the
opt-in, rotating every position against the model's own instruction. The
same shape as `use_sliding_window`, one key over, and found the same way.
Scoped to `granitemoehybrid` alone. `granite`, `granitemoe` and
`granitemoeshared` construct their rotary unconditionally and never
mention the key, so a family-wide rule would turn every dense Granite
into a NoPE model — a far larger wrong answer than the one being fixed.
`control_a_dense_granite_still_rotates_without_the_key` is that bound.
An out-of-contract value takes the same path as absence, because that is
what the reference does: HF compares against `"rope"` and sends every
other string down the `else` branch. The BERT lineage spells `absolute`
in this same leaf, so the case is not hypothetical.
Calibrated rather than assumed — with the check disabled, the adversarial
arms fail with `Rope { theta: 10000000.0 }` on all four layers, which is
the bug stated in its own output. Three control arms stay green.
Carried at `Represented`, not `Parsed`: the effect lands on every layer's
`PositionPolicy`, so `probe_position_embedding_type` asks the graph what
it believes rather than trusting the parser to have read the key. A stack
that rotates nowhere answers null — an answer, not a failure to answer.
`position_policy_for_layer`'s default body lifts to a free function so
the override can narrow the decision and then defer to it. Rust gives an
override no way to call the default it replaced, and copying the body
would leave two resolvers to keep in agreement.
Census, from cached headers:
ibm-granite/granite-4.0-micro 16 -> 15 blocking
ibm-granite/granite-4.2-3b 0 blocking (unchanged)
Qwen3 x6, all sizes 0 blocking (unchanged)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The first fix of census experiment #2 — and the recorded inspection for it was wrong.
What the census said, and what the reference says
position_embedding_type: "rope"on granite-4.0-micro was classified as a checked default that "restates the scheme already in use". Checking it againsttransformersinstead of against its name says the opposite:The default is no positional encoding at all.
"rope"is the opt-in that turns rotation on, not a restatement of anything.rope_thetais declared either way — granite-4.0-micro ships10000000— so a resolver that reads the theta and rotates is right about this checkpoint by luck, and wrong about anygranitemoehybridthat omits the opt-in: it would rotate every position against the model's own instruction. That is theuse_sliding_windowbug one key over, and it was found the same way.Scope, and the control that bounds it
Only
granitemoehybridgates rotation on this key.granite,granitemoeandgranitemoesharedconstruct their rotary unconditionally and never mention it, so a family-wide rule would turn every dense Granite into a NoPE model — a far larger wrong answer than the one being fixed.control_a_dense_granite_still_rotates_without_the_keyis that bound, and it covers all threemodel_types.An out-of-contract value takes the same path as absence, because that is what the reference does — HF compares against
"rope"and sends every other string down theelsebranch. Not hypothetical: the BERT lineage spellsabsolutein this same leaf, and the same key appears in 23 model families.Calibrated, not assumed
With the check disabled, the two adversarial arms fail with
Rope { theta: 10000000.0 }on all four layers — the bug stated in its own output — while all three control arms stay green. Red first, then green.Carriage
Represented, notParsed. The effect lands on every layer'sPositionPolicy, soprobe_position_embedding_typeasks the graph what it believes rather than trusting the parser to have read the key. A stack that rotates nowhere answersnull— an answer, not a failure to answer, the same wayprobe_sliding_windowreports "no window anywhere".position_policy_for_layer's default body lifts to a free function so the override can narrow the decision and then defer to it. Rust gives an override no way to call the default it replaced, and copying the body would leave two resolvers to keep in agreement.Census
From cached headers, no payload bytes:
granite-4.0-micro's remaining
layer_typesmismatch is pre-existing and unrelated — it is a hybrid Mamba/attention stack whose 40 declaredattentionlayers resolve to a different kind.Gates
fmt --all --check·check --workspace --all-targets(a newModelConfigfield, solarql-server's exhaustive literal matters) · clippy with each touched crate's own CI flags · full suites for larql-models, larql-vindex, larql-server · coverage policy for larql-models and larql-vindex, both re-measured afterllvm-cov clean.