Repository navigation
model, gpu: the Qwen plans count the stage rope table, and the placed loads hold it to the plan - #232
Conversation
github-actions
Bot
added
gpu
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: MUST_FIX: the new RoPE accounting invalidates 19 placement gate rows.
Must fix
1. Update the card-expert placement pins
crates/model/src/arch/qwen35moe/place.rs:1606 adds 8,388,608 B at context 32,768. The unchanged A6000 Q4 pin at ubatch 4,096 has only 2,805,184 B of slack. Its expected expert split cannot fit; hw_qwen4exp_card_plan fails. The Q3 table also needs updates.
Cost: restore the owning gate before merge.
Fix: replace both tables with these [derived] values. The 48-layer, 2 MiB allocator model reproduces all 32 original rows. Verify with just gate-qwen4exp-meta.
// PIN(2026-10-15): [derived] the stage rope table consumes 256 B per position on each card.
const CARD_PLANS: [CardPlanRow; 16] = [
("A6000", 4096, false, 4096, 262, 40, 261, 39385907200, 594162176),
("A6000", 4096, false, 512, 280, 28, 279, 42056192000, 616620544),
("A6000", 4096, true, 4096, 244, 17, 243, 36607078400, 522961408),
("A6000", 4096, true, 512, 262, 15, 261, 39308595200, 514187776),
("A6000", 32768, false, 4096, 258, 7, 257, 38682316800, 475668992),
("A6000", 32768, false, 512, 275, 33, 274, 41319936000, 530792960),
("A6000", 32768, true, 4096, 238, 8, 237, 35676876800, 574456320),
("A6000", 32768, true, 512, 255, 39, 254, 38329856000, 612123136),
("3090", 4096, false, 4096, 93, 5, 92, 13855948800, 522088960),
("3090", 4096, false, 512, 110, 37, 109, 16512000000, 554586624),
("3090", 4096, true, 4096, 73, 35, 72, 10940108800, 589996544),
("3090", 4096, true, 512, 92, 15, 91, 13736243200, 486605312),
("3090", 32768, false, 4096, 87, 27, 86, 13020979200, 532877824),
("3090", 32768, false, 512, 105, 21, 104, 15710208000, 536392192),
("3090", 32768, true, 4096, 67, 44, 66, 10065203200, 584098304),
("3090", 32768, true, 512, 86, 2, 85, 12792320000, 547627520),
];
// PIN(2026-10-15): [derived] the Q3 file uses the same 256 B per position stage rope table.
const CARD_PLANS_Q3: [CardPlanRow; 16] = [
("A6000", 4096, false, 4096, 365, 1, 364, 39689241600, 444786176),
("A6000", 4096, false, 512, 388, 20, 387, 42240409600, 584264192),
("A6000", 4096, true, 4096, 338, 4, 337, 36753254400, 532840960),
("A6000", 4096, true, 512, 362, 28, 361, 39423027200, 553714176),
("A6000", 32768, false, 4096, 355, 40, 354, 38686745600, 625198592),
("A6000", 32768, false, 512, 381, 45, 380, 41532416000, 472271360),
("A6000", 32768, true, 4096, 328, 44, 327, 35751628800, 649468416),
("A6000", 32768, true, 512, 355, 6, 354, 38611942400, 486092288),
("3090", 4096, false, 4096, 128, 41, 127, 13939020800, 590878208),
("3090", 4096, false, 512, 154, 5, 153, 16694656000, 530083328),
("3090", 4096, true, 4096, 103, 22, 102, 11171097600, 512966144),
("3090", 4096, true, 512, 128, 16, 127, 13883801600, 488811008),
("3090", 32768, false, 4096, 121, 28, 120, 13146700800, 561114624),
("3090", 32768, false, 512, 146, 17, 145, 15848524800, 554130944),
("3090", 32768, true, 4096, 94, 33, 93, 10214579200, 588680704),
("3090", 32768, true, 512, 119, 36, 118, 12946867200, 549135872),
];2. Reuse the buffer's byte-count API
crates/gpu/src/rope_table.rs:239 duplicates DeviceBuffer::num_bytes(), already used for this table at crates/gpu/src/arch/qwen3moe/body.rs:1016. Both new callers are inside this crate; the public wrapper is also unnecessary under R13.
Cost: remove the extra API before it gains callers.
Fix: delete RopeRows::bytes and its documentation, and replace the two new calls:
// body.rs
refuse_stage_rows_past(what, open.plan, 0, rope.table.num_bytes() as u64)?;
// body35.rs
refuse_stage_rows_past(WHAT, open.plan, 0, rope.table.num_bytes() as u64)?;3. Remove Qwen3's redundant RoPE-width field
crates/model/src/arch/qwen3moe/place.rs:66 stores a second width. crates/model/src/arch/qwen3moe/hparams.rs:152 already rejects a RoPE width different from head_dim, which this layout retains.
Cost: avoid maintaining both fields in more constructors.
Fix: remove rope_row and its field documentation, remove its initializer, and use:
fn stage_position_bytes(&self, ctx_max: u64) -> u64 {
self.head_dim as u64 * F32_BYTES * ctx_max
}Non-blocking
⚠ Complete the refusal evidence: crates/gpu/src/arch/qwen3moe/body35.rs:1669. Publish the queued gates and zero/half-table FAIL-first outputs with the card/driver, both commits and model. Also check that deleting either new refusal makes an assertion fail.
Checked
Duplicates: 10 new functions and 2 fields checked; redundant elements above. Dryer reported 0 pairs.
Complexity: Body::load 24→25 and Body35::build 44→45 add load-time error propagation.
Silent failure: 0 new default/discard sites; missing-card lookup returns a named error.
Static mutation check: zero/half-row counts violate new unit assertions; dropping MTP stage totals violates headroom assertions. The byte calculation finds 19 stale rows. Gates were not run.
Gates: context boundaries and floors have dated PIN reasons.
Escape catalog: no patterns match.
Card check: needs-gpu. This pull request changes crates/gpu/src/arch/qwen3moe/body.rs, crates/gpu/src/arch/qwen3moe/body35.rs, crates/gpu/src/model.rs … and carries no gate run on a card (CONTRIBUTING's evidence list); a maintainer adds gpu-verified after one.
🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)
|
Gate results for 8dc1d54:
FAIL-first:
This comment was written by the bloomery AI assistant on behalf of the maintainer. |
… loads hold it to the plan The Qwen3 and Qwen3.6/3.8 bodies copy one rope table of rope.dimension_count f32 a position at load (body.rs, body35.rs RopeRows::new); their KvLayout impls now count it through KvBytes::stage_position_bytes, so the plan's CardTotals::stage_rows_bytes carries it on the target card. The MTP draft reads the target's table and keeps the trait's default; the slot plan counts one table for every slot. The placed loads hold the table they made to that count by name (model::refuse_stage_rows_past, the stage-row half of refuse_scratch_past, which the GLM and MiMo loads call): a plan that counts no table, or half of one, refuses the load. The qwen4exp plan tests' boundaries and floors move by the 256 B a position; their PIN lines carry the derivations. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CARD_PLANS and CARD_PLANS_Q3 take the derived rows for a 256 B stage rope table. Qwen3 counts that table from head_dim, and the placed loads read the buffer's own byte count.
8dc1d54 to
5a9d496
Compare
There was a problem hiding this comment.
Verdict: MUST_FIX: Unneeded public visibility and single-test fixture builders remain.
Must fix
1. Keep the refusal helper inside its crate
crates/gpu/src/model.rs:1951 Kind B, R13: the only callers are body.rs:947 and body35.rs:1669, both inside bloomery-gpu. Public visibility adds an external API the change does not need.
Narrowing it now avoids withdrawing an API after downstream use. Replace the declaration line:
pub(crate) fn refuse_stage_rows_past(2. Inline the fixture builders used by one test each
crates/model/src/arch/qwen3moe/place.rs:207 and crates/model/src/arch/qwen35moe/place.rs:2304 each add a layout builder for one consuming test. The first has one call at line 202; the second has two calls in the same test, at lines 2298 and 2301. Their head-width parameters never vary: 128 and 256 respectively. These are kind B unneeded elements under the fewest-elements rule.
Removing them now prevents later tests from depending on unused fixture generality. Replace each test and its layout function with its block below.
#[test]
fn the_stage_rope_row_is_the_width_the_body_copies() {
let kv = KvLayout::of(&Hparams {
n_layer: 1,
n_embd: 128,
n_head: 1,
n_head_kv: 1,
head_dim: 128,
rope: Rope {
mode: RopeMode::Neox,
base: 1e6,
dims: 128,
},
rms_eps: 1e-6,
n_vocab: 2,
n_ctx_train: 128,
experts: Experts {
n_expert: 1,
n_used: 1,
ff: 1,
score: Score::Softmax,
weights_norm: true,
},
});
assert_eq!(kv.stage_position_bytes(10), 128 * 4 * 10);
assert_eq!(kv.in_q8().stage_position_bytes(10), 128 * 4 * 10);
}#[test]
fn the_stage_rope_row_is_rope_dims_not_the_head() {
use super::super::hparams::{FfnKind, Hparams, SectionOrder, Variant};
let hp = Hparams {
variant: Variant::Qwen35Moe,
n_layer: 1,
n_trunk: 1,
n_embd: 256,
n_head: 1,
n_head_kv: 1,
head_dim: 256,
rope_dims: 64,
rope_sections: [0, 0, 0, 0],
rope_order: SectionOrder::Tyxz,
rope_base: 1e7,
rms_eps: 1e-6,
n_vocab: 2,
n_ctx_train: 8,
n_expert: 0,
n_used: 0,
expert_ff: 0,
shared_ff: None,
ff: None,
conv: 4,
state: 1,
v_heads: 1,
k_heads: 1,
interval: 1,
kinds: vec![Kind::Attention],
ffns: vec![FfnKind::Dense],
exp: None,
defaults: Vec::new(),
};
let narrow = KvLayout::of(&hp);
assert_eq!(narrow.stage_position_bytes(8), 64 * 4 * 8);
assert_eq!(narrow.in_q8().stage_position_bytes(8), 64 * 4 * 8);
assert_eq!(
KvLayout::of(&Hparams { rope_dims: 128, ..hp }).stage_position_bytes(8),
128 * 4 * 8
);
}Checked
Elements: checked both fixture builders' consumers and the refusal helper's callers. The inline suggestion changes an added declaration line and preserves its signature.
Complexity: the two placed-load branches call the common refusal through ?.
Silent failure: the new lookup returns a named error for a missing card; the refusal returns a named error when the table exceeds the count.
Tests: by inspection, zero/half-row mutants fail the new literal-value assertions. Rust/GPU gates were not run.
Gates: changed budget pins carry PIN(2026-10-15) and byte derivations.
Escape catalog: no C1–C4 pattern matched.
Notes
The PR lists gates and FAIL-first runs as queued. No card/driver line or gate and mutant command output is supplied.
Card check: needs-gpu. This pull request changes crates/gpu/src/arch/qwen3moe/body.rs, crates/gpu/src/arch/qwen3moe/body35.rs, crates/gpu/src/model.rs and carries no gate run on a card (CONTRIBUTING's evidence list); a maintainer adds gpu-verified after one.
🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)
| /// (`CardTotals::stage_rows_bytes`, the rope tables at the context): the check | ||
| /// of a body whose scratch the plan holds by a term of its own rather than | ||
| /// through [`refuse_scratch_past`]. | ||
| pub fn refuse_stage_rows_past( |
There was a problem hiding this comment.
R13: the only callers are body.rs:947 and body35.rs:1669 in this crate; use crate visibility.
| pub fn refuse_stage_rows_past( | |
| pub(crate) fn refuse_stage_rows_past( |
…est builds its layout refuse_stage_rows_past is crate-visible. The two stage-rope tests no longer keep a layout helper that one test calls.
There was a problem hiding this comment.
Since the last review:
- Must-fix 1 fixed: crates/gpu/src/model.rs:1951 now uses
pub(crate). Both callers are inside the crate. - Must-fix 2 fixed: the fixture builders are inlined at crates/model/src/arch/qwen3moe/place.rs:198 and crates/model/src/arch/qwen35moe/place.rs:2294, preserving their inputs and assertions.
Verdict: CLEAN: Both earlier findings are fixed, and the new commits introduce no further findings.
Checked
Duplicates: 0 new items; two fixture builders removed. Dryer reported 0 pairs.
Complexity: crapper flagged Body::load (24 → 25) and Body35::build (44 → 45), each from an added refusal call using ? in the earlier changes. Neither function changes in this incremental diff.
Silent failure: 0 new sites. The visibility change preserves both callers and the existing error propagation.
Tests: by inspection, returning zero or half the row bytes at qwen3moe/place.rs:101 or qwen35moe/place.rs:1497 fails the retained literal assertions. Using head width instead of rope dimensions in the latter also fails. The layout owns its fields, so moving hp into the second fixture leaves no borrowed state. Rust/GPU tests were not run.
Gates: no pins, thresholds or assertions changed in the new commits.
Escape catalog: no C1–C4 pattern matched.
Notes
The PR still lists gate and FAIL-first runs as queued. No card/driver line or gate and mutant command output is supplied.
Card check: needs-gpu. This pull request changes crates/gpu/src/arch/qwen3moe/body.rs, crates/gpu/src/arch/qwen3moe/body35.rs, crates/gpu/src/model.rs and carries no gate run on a card (CONTRIBUTING's evidence list); a maintainer adds gpu-verified after one.
🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)
contrib-check t0 stopped on the one-line Hparams update in the qwen3.5 fixture.
There was a problem hiding this comment.
Since the last review:
- Earlier must-fix 1 remains fixed: the refusal helper uses
pub(crate)at crates/gpu/src/model.rs:1951. - Earlier must-fix 2 remains fixed: fixtures stay inlined at crates/model/src/arch/qwen3moe/place.rs:198 and crates/model/src/arch/qwen35moe/place.rs:2294.
Verdict: CLEAN: The incremental diff only reformats one test expression and preserves its behavior.
Checked
- Duplicates: 0 new items; dryer reported 0 pairs.
- Complexity: crapper flagged
Body::load(24 → 25) andBody35::build(44 → 45) for earlier refusal calls using?; neither function changes here. - Silent failure: 0 new sites.
- Tests: replacing
self.rope_row * ctx_maxat crates/model/src/arch/qwen35moe/place.rs:1607 with zero fails the retained assertions by inspection. Removed and added expression text is identical after whitespace removal. Rust/GPU tests were not run. - Gates: no pins, thresholds, cases or assertions changed in the incremental diff.
- Escape catalog: no C1–C4 pattern matched.
Notes
The PR body still lists gate and FAIL-first runs as queued. It supplies no card/driver line or gate command output.
Card check: needs-gpu. This pull request changes crates/gpu/src/arch/qwen3moe/body.rs, crates/gpu/src/arch/qwen3moe/body35.rs, crates/gpu/src/model.rs and carries no gate run on a card (CONTRIBUTING's evidence list); a maintainer adds gpu-verified after one.
🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)
1 participant
The Qwen3 and Qwen3.6/3.8 bodies copy one rope table at load:
rope.dimension_countf32 per position (RopeRows::newinarch/qwen3moe/body.rsandbody35.rs). TheirKvLayoutimpls now count it throughKvBytes::stage_position_bytes, soCardTotals::stage_rows_bytescarries it on the target card. That is 512 B per position for Qwen3 (HEAD 128) and 256 B for Qwen3.6/3.8 (ROPE_DIMS 64).model::refuse_stage_rows_past, the stage-row half ofrefuse_scratch_past(the GLM and MiMo loads call the latter), and both share one lookup. A plan that counts no table, or half of one, refuses the load.PIN(2026-10-15)lines carry the derivations:CARD_MAX_CTXA6000 1,398,249 / 3090 505,321;CARD_MAX_CTX_MTP1,212,599 / 379,200.Change class: the plan bytes plus a load-time refusal. No kernel or launch changed.
Pre-review:
pre-review: CLEAN — full review of 8dc1d542 against origin/mainMac static (
tools/mac-remote.sh origin/main): ok, every step rc 0.Gates (queued; results will follow in a comment):
stage_position_bytesreturns 0, under the fixture e2estage's position rows hold🤖 Generated with Claude Code