Skip to content

feat(qwen4): score QSA token-indexer blocks in native CUDA - #1160

Merged
FeathBow merged 12 commits into
pegainfer-project:mainfrom
sysuTobo:feat/qwen4-qsa-indexer-score
Oct 20, 2026
Merged

FeathBow merged 12 commits into
pegainfer-project:mainfrom
sysuTobo:feat/qwen4-qsa-indexer-score

Conversation

@sysuTobo

@sysuTobo sysuTobo commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

This lands Stage 1 / P2 of the QSA (sparse attention) bring-up for the Qwen3.8-Flash-Next line: the token-indexer block-scoring kernel in native CUDA, plus the parity gate that replays the committed reference golden. Selection is P3 and out of scope here.

Summary

pegainfer-kernels/csrc/qwen4/qwen4_qsa_indexer_score.cu — two kernels behind an additive qwen4 feature on pegainfer-kernels, so the model crate pegainfer-qwen4 keeps having no CUDA dependency at all (that is the point of its shape, and #1149's CPU-only test set is untouched). No PyTorch, no new build-time dependency, BF16 only.

The gate is pegainfer-kernels/tests/qwen4_qsa_indexer_score_parity.rs: it replays test_data/qwen38fn-qsa-indexer-golden.safetensors (merged in #1150) through the kernel and compares block scores against the reference. Two trap binaries pin the kernel's out-of-range behavior (qwen4_qsa_indexer_{rows,visible}_trap.rs).

file what
pegainfer-kernels/csrc/qwen4/qwen4_qsa_indexer_score.cu the two kernels + one extern "C" entry point
pegainfer-kernels/src/ffi/qwen4.rs the FFI declaration
pegainfer-kernels/src/ops/qwen4.rs qsa_indexer_block_scores_into and the compiled-geometry constants its buffer checks use
pegainfer-kernels/tests/qwen4_qsa_indexer_score_parity.rs the gate: two device-free halves + the device comparison + the NaN gate
pegainfer-kernels/tests/qwen4_qsa_indexer_rows_trap.rs out-of-range rows traps (one trap per binary)
pegainfer-kernels/tests/qwen4_qsa_indexer_visible_trap.rs out-of-range visible_len traps
pegainfer-kernels/{Cargo.toml,build.rs} + src/{ffi,ops}.rs the additive qwen4 feature and its directory gate (4 small edits, no existing row touched)
docs/models/qwen4/bring-up.md what landed, the next step, and what the gate measured

Kernel design, and the decisions it follows

Frozen semantics are transformers 5.16.1 qwen4_exp, Qwen4ExpTextQSAIndexer.forward, per visible micro-block b of a query row at position p:

q_h          = (1 + q_layernorm_w) RMSNorm128(qk_q_h), partial RoPE at p
pooled_key_b = mean(keys[4b .. 4b+4)) in float32, cast back to bf16,
               then (1 + k_layernorm_w) RMSNorm128, RoPE at 4b
score_b      = sum_{h < 4} relu(q_h . pooled_key_b) / sqrt(128)
  • The four query heads share one key head (indexer_kv_heads = 1), and the relu is per head before the sum over heads — both visible in the structure, not just the arithmetic.
  • Every bf16 rounding point of the reference is reproduced: the norm's single cast after a float32 scale, the pooled mean's round trip through bf16 before the norm (so the norm squares the rounded value), and the bf16 rotation (cos/sin read from the shared attention table, already bf16 like the reference's cast, each product and their sum rounded). The gate holds cases 0–5 to a 2e-6 bound, and each of those decisions, when flipped, moves scores by 3.3e-3 to 2.7 — so none of them can slip through (measurements under Tolerance).
  • Two passes: pooling does not depend on the query row (the reference rebuilds the same numbers per row), so one pass over the keys serves every row, and the per-row pass does its own query prep plus the score reduction. The pooled half is the workspace argument, so no allocation happens inside the crate.
  • Grid contract: the reference groups the visible list from its first visible token, so this kernel reads the key array as starting at token 0 (causal text, no prefix eviction) and asserts it in the signature by taking visible_len separately from rows. Nothing in the kernel can detect a visible list anchored elsewhere; that caller must hand it a shifted key array.
  • [Model] Qwen3.8-Flash-Next: QSA sparse attention #1105 D2 (block/page layout): the page_size = 16 / compress_ratio = 4 divisibility is taken as given and not used by this stage, which reads the raw key array; what this stage does depend on from D2 is the grid anchor (token 0) above.
  • [Model] Qwen3.8-Flash-Next: QSA sparse attention #1105 D3-A/C: native CUDA in a model-local directory (csrc/qwen4/, named after the model line like qwen35 and glm52), no vendored FlashInfer change — the FlashInfer submodule is untouched, so no new submodule patch to maintain.
  • [Model] Qwen3.8-Flash-Next: QSA sparse attention #1105 D3-B: the sm_89-first portable subset — no TMA, no cluster, no wgmma. Both kernels are SIMT shuffle reductions rather than mma.sync bodies, because one block's score is a 128-wide dot over 4 query heads, a shape m16n8k16 cannot tile without padding 4 rows to 16. No mma/cp.async is used at all; the exclusion list is what this variant obeys.

The kernel is compiled for one geometry; the wrapper carries only rms_eps at run time. The fused row is 640 wide, the shared tables stride 64, and four tokens make a micro-block; the wrapper sizes its buffer checks from those numbers. The checkpoint's own indexer fields are checked where the model crate reads its config, once that caller lands — this stage does not carry a second geometry object to pin, and the score comparison is what holds the compiled kernel to the config. rows and visible_len are caller-provided device values: an out-of-range entry traps on the device (the checked-row pattern of common.cuh, no D2H copy), while the valid empty prefix — visible_len = 0 — still succeeds and writes nothing. The relu is written as a comparison rather than fmaxf, so a poisoned key keeps its NaN the way torch.relu does, instead of turning into a valid-looking lowest score.

Evidence

Exact commands, from this worktree, in the dev container (./docker/dev.sh run ..., image pegainfer-dev:cu132, one sm_120 GPU):

$ ./docker/dev.sh run cargo test --release --locked -p pegainfer-kernels --features qwen4 \
    --test qwen4_qsa_indexer_score_parity -- --nocapture
running 3 tests
golden provenance: transformers 5.16.1, torch 2.14.0, arm64
test fixture_records_the_frozen_reference ... ok
golden provenance: transformers 5.16.1, torch 2.14.0, arm64
golden provenance: transformers 5.16.1, torch 2.14.0, arm64
poisoned key: block 0 is NaN on 61 rows, later blocks finite
test a_poisoned_key_keeps_its_nan_score ... ok
case 0: 2 block scores over 5 rows, tokens 5, budget 2048, blocks scored 1 vs kept 512: max abs 8.941e-8 at row 4 block 0 (0.399860233 against 0.399860144); worst bound fraction 0.032 at row 4 block 0 (0.399860233 against 0.399860144); bound ATOL 2.0e-6 + RTOL 2.0e-6 * |want|; above 1e-4 0/2 (0 positive), above 1e-5 0/2, bias +5.96e-8, widest row 0 with 0
case 1: 6 block scores over 8 rows, tokens 8, budget 2048, blocks scored 2 vs kept 512: max abs 4.768e-7 at row 3 block 0 (4.939687252 against 4.939687729); worst bound fraction 0.072 at row 4 block 0 (1.469320774 against 1.469320416); bound ATOL 2.0e-6 + RTOL 2.0e-6 * |want|; above 1e-4 0/6 (0 positive), above 1e-5 0/6, bias +5.96e-8, widest row 0 with 0
case 2: 496 block scores over 64 rows, tokens 64, budget 2048, blocks scored 16 vs kept 512: max abs 9.537e-7 at row 38 block 6 (7.821538925 against 7.821537971); worst bound fraction 0.135 at row 51 block 10 (1.643389583 against 1.643390298); bound ATOL 2.0e-6 + RTOL 2.0e-6 * |want|; above 1e-4 0/496 (0 positive), above 1e-5 0/496, bias -4.54e-9, widest row 0 with 0
case 3: 21 block scores over 14 rows, tokens 14, budget 16, blocks scored 3 vs kept 4: max abs 9.537e-7 at row 7 block 0 (4.586654663 against 4.586655617); worst bound fraction 0.085 at row 7 block 0 (4.586654663 against 4.586655617); bound ATOL 2.0e-6 + RTOL 2.0e-6 * |want|; above 1e-4 0/21 (0 positive), above 1e-5 0/21, bias +3.97e-8, widest row 0 with 0
case 4: 28 block scores over 16 rows, tokens 16, budget 16, blocks scored 4 vs kept 4: max abs 9.537e-7 at row 13 block 1 (9.966370583 against 9.966371536); worst bound fraction 0.080 at row 8 block 1 (3.481083632 against 3.481084347); bound ATOL 2.0e-6 + RTOL 2.0e-6 * |want|; above 1e-4 0/28 (0 positive), above 1e-5 0/28, bias -6.65e-8, widest row 0 with 0
case 5: 45 block scores over 20 rows, tokens 20, budget 16, blocks scored 5 vs kept 4: max abs 7.153e-7 at row 15 block 2 (3.094076395 against 3.094077110); worst bound fraction 0.087 at row 15 block 2 (3.094076395 against 3.094077110); bound ATOL 2.0e-6 + RTOL 2.0e-6 * |want|; above 1e-4 0/45 (0 positive), above 1e-5 0/45, bias -7.62e-9, widest row 0 with 0
case 6: 3076 block scores over 6 rows, tokens 2055, budget 2048, blocks scored 513 vs kept 512: max abs 2.401e-3 at row 2051 block 257 (3.210482359 against 3.212882996); worst bound fraction 0.164 at row 2050 block 257 (1.746432543 against 1.748687387); bound ATOL 5.0e-3 + RTOL 5.0e-3 * |want|; above 1e-4 152/3076 (76 positive), above 1e-5 235/3076, bias -5.20e-7, widest row 2054 with 147
test qsa_indexer_block_scores_match_the_reference ... ok
test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
$ ./docker/dev.sh run cargo test --release --locked -p pegainfer-kernels --features qwen4 \
    --test qwen4_qsa_indexer_rows_trap
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

$ ./docker/dev.sh run cargo test --release --locked -p pegainfer-kernels --features qwen4 \
    --test qwen4_qsa_indexer_visible_trap
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

$ ./docker/dev.sh run cargo test --release --locked -p pegainfer-kernels --features qwen4 --lib
test result: ok. 19 passed; 0 failed; 6 ignored; 0 measured; 0 filtered out

$ ./docker/dev.sh run cargo fmt --all -- --check                              # clean
$ ./docker/dev.sh run cargo clippy --release --locked -p pegainfer-kernels \
    --features qwen4 --all-targets -- -D warnings                             # no findings

What the gate asserts, beyond the score comparison: the fixture's format_version / transformers_version / config / config_sha256 against this build's frozen config, and that config's indexer geometry (indexer_n_heads 4, indexer_kv_heads 1, indexer_head_dim 128, indexer_compress_ratio 4, rope_theta 1e7, rms_norm_eps 1e-6, rotary_dim = head_dim × partial_rotary_factor = 64) against QsaIndexerGeometry::qwen38_flash_next(); per row it compares only the fixture's explicit validity prefix (row + 1) / compress_ratio (the reference NaN-pads the rest) and asserts the NaN padding is really there; and it asserts the kernel left every entry past that prefix holding the sentinel it was pre-filled with, so a wrong block count cannot pass as a shorter comparison. One fixture property is still asserted because this stage depends on it — at least one case must carry an incomplete tail; the budget and block-count bookkeeping is selection's business and is printed, not asserted.

The device-free half

fixture_records_the_frozen_reference needs no device and is not behind device_or_skip, so it cannot be skipped: it holds the fixture's recorded format_version, transformers_version and config_sha256 against the frozen config. The gate then derives its geometry from that config and cross-checks the numbers against the fixture's own tensor shapes, so a checkpoint or config drift reaches the kernel as a score mismatch rather than as a Rust-side duplicate of the constants. (An earlier revision also carried a device-free refusal test for a QsaIndexerGeometry struct; review pointed out that it only compared the literal to itself, and both are gone.)

Tolerance: 2e-6 on cases 0–5, 5e-3 on the one case whose residual is untraced

abs(got - want) <= ATOL + RTOL * abs(want), with 2e-6 / 2e-6 on cases 0–5 and 5e-3 / 5e-3 on the 2055-token case.

  1. Cases 0–5 are exact at the bf16-decision level: max 9.537e-7 over their scores (8.9e-8, 4.8e-7 and 9.5e-7 by case), i.e. within 1–2 f32 ulps of scores of order 1–10.
  2. What the bound must catch was measured by mutation audit (one kernel change at a time, gate unchanged): relu after the head sum 166x the bound, pooled-key RoPE at the block end 536x, mean kept in f32 3.1x, rms_eps ×100 2.5x — and, under the old single 5e-3 bound, pooling by sum and dropping rms_eps both passed at 0.66x. The last two are the ones the tight bound exists for: they move scores by 3.3e-3 (the pooled key's RMSNorm is scale-invariant, so pooling by sum differs from the mean only through eps at the pooled key's scale), which is ~1600x the new 2e-6 bound. With the split bound every measured fault now fails clearly.
  3. The long case is the only one with positions past 63, and its 2.4e-3 residual has the shape of a flipped rounding decision, not a semantic difference: 147 of its 152 deviating scores sit in a single row — one flipped query element moves every score that row touches — the sign is balanced (76 of 152 positive), and the bias over 3076 scores is -5.2e-7. Its magnitude is the reference's own floor: re-deriving case 2 with the reference's own ops, from the fixture's own inputs and weights (torch 2.14.1+cpu, against a golden dumped under torch 2.14.0 on arm64 — the gate prints both), moves one of its 496 scores by 2.2e-3, the same single-flip size, while on that case this kernel is three orders closer (9.5e-7). It keeps 5e-3 until that residual is traced.
  4. Architecture variance is not the reason for any bound here. sm_89 and sm_120 print identical numbers, and the kernel now reads the attention line's shared cos/sin table (pegainfer_core::rope, built once and passed in) instead of computing trig, so there is no device-specific surface left to vary — the table-driven run reproduces the previous scores bit for bit. The short cases' 2e-6 is calibrated to the measured mutation floor, not to a cross-host drift.

Why this gate does fail if the kernel is wrong. The fixture measures the distance itself: a one-block grid shift replaces each score with its neighbour's, and over the fixture's own reference scores that is a median |delta| of 1.49 (p10 0.26, p90 3.78) at the long case — 166x the bound at the typical score for the measured relu fault, 536x for the RoPE anchor, on nearly every element. The same order covers a missing sqrt(128), summing the heads before the relu, rotating 128 dims instead of 64, and reading the key head at the wrong fused column; the rounding faults measured in 2 are now two to three orders past the tight bound.

Diagnostics, not assertions. The sign split, the bias and the widest row are printed as evidence about the residual's shape, not asserted: independent flips cancel in a mean, and a scale error cancels too (it flips the residual's sign with the score's), so such an assertion would add a brittle threshold with little power. The assertion is the bound, and each of max abs and max bound fraction now prints its own element's coordinates.

Kernel limitations / what is NOT verified

  • Perf is not claimed. No benchmark, no bandwidth numbers. [Model] Qwen3.8-Flash-Next: QSA sparse attention #1105 leaves the sm_90 perf variant (TMA/wgmma) as a later item, and D3-B is a correctness-first portable subset; the shuffle-reduction-heavy shape is a perf decision left open, not measured.
  • No budget, no selection, no tail. The kernel scores every complete block; the incomplete tail is neither pooled nor scored. The budget and its divisibility are selection's business and are not carried here.
  • The fused index_qk_proj (2560 → 640) is a GEMM, not part of this kernel, and the golden starts after it (its docstring says so): this stage gates the semantics from the projection's output onward.
  • The visible list is assumed to start at token 0. A prefix-evicted or otherwise shifted visible list is not served and cannot be detected here (see the grid contract); such a caller gets a shifted grid and a plausible-looking wrong score. The line does not evict prefixes today, but that is slice B's runtime and not verified by this gate.
  • Out-of-range rows / visible_len trap on the device rather than being clamped: an in-range contract is asserted, the valid empty prefix still succeeds, and both trap paths have their own binaries. A poisoned key keeps its NaN score (torch.relu semantics), gated by a_poisoned_key_keeps_its_nan_score.
  • tokens < 4 (no complete block in the whole buffer) is partly covered: rows 0–2 of case 0 exercise the per-row empty prefix, and the whole-buffer zero extent is pinned by the entry-point test (which asserts it returns success and forms no launch), but no score comparison covers it.
  • Chunked prefill (the reference's indexer key cache spanning past + current) is not covered: every case here hands the kernel one contiguous prompt. The pass takes whatever key array it is given, but only the single-chunk shape is gated.
  • sm_89 and sm_120 were both exercised (the review on sm_89, this worktree on sm_120) and print identical gate numbers; sm_80 compilation is not verified. No CI job enables the qwen4 feature — the CUDA-clippy matrix names pegainfer-kernels only on the default-features row, so every guard in this PR (the two device-free halves, the ops unit test and the traps) is compiled by CI only as an empty target and runs today only under the manual commands above. Wiring a row for this line's kernels is a follow-up, and it must set PEGAINFER_REQUIRE_GPU=1 so the device half cannot skip and still report green.
  • No production E2E is possible yet. P2's gate is component-wise by construction: pegainfer-qwen4 has no runtime, launch refuses, and Stage 2/4 (sparse attention forward, assembly into the layer) do not exist — no request path reaches this kernel. The golden parity is the whole of the evidence, and the nvcc does not list compute_120f; compiling CUDA kernels for raw sm_120 warning is expected, not a failure.
  • CUDA-graph capture is Stage 3's question; nothing here was captured or replayed.

Next: P3

Selection — topk(min(budget / compress_ratio, n_complete)) over the block scores, expanding each kept block back to its compress_ratio tokens, then appending the unconditional len(visible) % compress_ratio tail (buffer upper bound budget + compress_ratio - 1 = 2051). It depends on the same golden's case*_mask (already in the fixture) and not on this kernel. The reference leaves tied scores' order open, so a block tied with the last kept one at the cut is an equally valid pick — the long case's bf16 noise documented above is exactly the perturbation that can reach that tie, which is worth keeping in mind when that gate picks its own bound.

Refs

#1105 (QSA), #1150 (the golden this gate replays), #1103 / #1104 (the line)

QSA's token indexer is the highest-lead-time piece of this line's bring-up, and
it gates on a golden that has been on `main` since pegainfer-project#1150 with nothing to replay
it. This lands the middle of the three units pegainfer-project#1105 splits QSA into: the scoring
half, in `pegainfer-kernels/csrc/qwen38fn/` behind the kernels crate's own
`qwen4` feature, so the model crate stays free of CUDA.

Two kernels against the frozen transformers-5.16.1 semantics. One pools every
complete micro-block of `indexer_compress_ratio` tokens into a single key — the
block's raw keys averaged in float32, cast back to bf16 as the reference casts
them, then `(1 + w)` RMSNorm and partial RoPE at the block's first token — and
the other norms each of a query row's four heads at the row's own position and
reduces `sum_h relu(q_h . k_b) / sqrt(indexer_head_dim)`. The pooled half is a
pass of its own because it does not depend on the query row: the reference
rebuilds it per row from the same keys through the same bf16 rounding points, so
one pass over the keys is the same numbers for every row that can see them.

It follows pegainfer-project#1105's D3: A/C native CUDA in a model-local directory rather than a
vendored FlashInfer patch, and B the sm_89-first portable subset — no TMA, no
cluster, no wgmma. Both kernels are SIMT shuffle reductions rather than
`mma.sync` bodies, because one block's score is a 128-wide dot over 4 query
heads: a shape m16n8k16 cannot tile without padding 4 rows to 16.

The gate replays `test_data/qwen38fn-qsa-indexer-golden.safetensors` and holds
its recorded `format_version` / `transformers_version` / `config_sha256`, plus
the frozen config's own indexer geometry, against this build — a golden from
another checkpoint fails loudly instead of skipping. It reads each row's
explicit validity prefix rather than comparing through the reference's NaN
padding, and proves the kernel writes exactly that prefix by checking the
untouched tail still holds a sentinel. Six of seven cases land within 9.6e-7 of
the reference; the 2055-token case, the only one with positions past 63, within
2.4e-3 of its 5e-3 bound.

Selection (top-k over blocks plus the unconditional tail) is pegainfer-project#1105's next unit,
which is why this kernel takes no budget: the reference scores every complete
block and truncates only at selection.

Refs pegainfer-project#1105

Signed-off-by: bohaotu <bohaotu@pony.ai>
The entry point of the QSA indexer's scoring kernel validates before it forms a
launch, so its half of the refusal contract is pinnable with no device — and
nothing pinned it. It is the only place the compiled row stride is checked,
which is the reason `fused_width` is an argument at all: the body reads
`fused_qk` with the width it was compiled for, so a caller whose rows are
strided differently is read out of bounds rather than computed wrongly, the one
mismatch the pointer arithmetic cannot survive. `rotary_dim` and `rope_theta`
are compiled constants with no other check anywhere.

The new test calls the entry point with the compiled geometry and a zero extent
first, so the guards are the whole of the call and no launch follows: that
positive control is what keeps every refusal below from being vacuous. Then each
refusal: a row stride either side of 640, a rotary split of 32 and of 128, a
rope_theta of 1e6 and one f32 ulp off 1e7, a negative and a NaN rms_eps, three
negative extents, a score stride under the block grid, and one null per kind of
argument. The pointers dangle and are never dereferenced, because a launch is
the only thing that would touch them.

Also prints the golden's recorded torch and platform in the provenance half. The
parity bound's justification leans on the dumping host's reduction order, which
was otherwise invisible to whoever reads this gate's output.

Refs pegainfer-project#1105

Signed-off-by: bohaotu <bohaotu@pony.ai>
@github-actions github-actions Bot added qwen4 Qwen4-architecture model crate (pegainfer-qwen4) enhancement New feature or request labels Oct 14, 2026
@sysuTobo

sysuTobo commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor Author

Code review

覆盖 10 个文件(2 个 CUDA/FFI/ops 新文件 + 门禁 + build/Cargo 接线 + bring-up 文档),无遗漏;三条数据流的正向路径全部符合预期:

  • DF1 键 → 池化键 → 分数:fused_qk[t*640 + 512 + d](csrc/qwen38fn/qwen38fn_qsa_indexer_score.cu:138)→ 4 token 的 fp32 均值 → bf16 往返(:142)→ (1+w) RMSNorm(:145-154)→ 块首位置的 partial RoPE(:155-157)→ 每行取 dot + relu + /sqrt(128)(:247-277)。份量:2055 token → 513 块 → 池化工作区 513×128 bf16 = 131 KB;分数 6×514 f32。
  • DF2 行 → 分数:rows[row] 取位置与读行(:187)、visible_len[row] 定块数(:193-205)→ 写入 scores[row*score_cols + block]。写界有界:row_blocks ≤ blocks ≤ score_cols(:340 与 ops 侧 :162-166 双向把关),越界输入被 clamp 而非越读。
  • DF3 几何:frozen_config.json → QsaIndexerGeometry::qwen38_flash_next()(src/ops/qwen38fn.rs:53-64)→ entry point 的编译期常数(csrc/...:327-333);门禁把 config 的 indexer 字段与 Rust 几何逐项对齐(tests/...:159-201),链路闭合。
  • 单线程驱动多设备 / host 阻塞点 / 失败路径时序三节不适用:这是单个 kernel,无 rank 协同、无 host 阻塞点、无集合通信。
  • 文档 claim 与实测一致:docs/models/qwen4/bring-up.md 的「六例 ≤9.6e-7、2055-token 例 2.4e-3、152 of 3076」与本次运行及 fixture 形状逐项核对相符(fixture 单例 compared 3076、总计 3674)。
ID 级别 一句话 证据(文件:行)
R1 M 内核侧 refusal 契约(fused_width/rotary_dim/rope_theta/rms_eps、负 extent、空指针、blocks > score_cols)此前无任何测试:Rust 侧 validate() 先拒,直接调 FFI 的那半没人守 csrc/qwen38fn/qwen38fn_qsa_indexer_score.cu:319-343;tests/qwen38fn_qsa_indexer_score_parity.rs:30
R2 M 没有任何 CI job 启用 qwen4 feature,三处 device-free 守卫在 CI 中一次都不编(Qwen3 行只带 -p pegainfer-kernels,不带 feature) .github/workflows/ci.yml:249-267、:319-324;tests/...:30
R3 L validate() 中 rotary_dim > 0 && <= head_dim、compress_ratio > 0 恒不可达:已被前面的精确几何 pin 覆盖 src/ops/qwen38fn.rs:84-92 vs :105-120
R4 L 残差的符号平衡与 bias 只打印不断言,而容差论证正是靠它 tests/...:373、:429、:503、:749
R5 L 容差注释引用了 golden 的 dump 主机(torch 2.14.0 / arm64),门禁不记录该事实 tests/...:66-83
R6 L 冻结几何有两份真相来源(内核 constexpr 与 Rust 字面量),运行期守卫只覆盖 3 个量,kRatio 漂移只能靠门禁兜 csrc/...:52-59 vs src/ops/qwen38fn.rs:54-63,84-99
R7 L 无 GPU 时只有设备那一半被跳过;将来 CI 若不设 PEGAINFER_REQUIRE_GPU=1 会静默跳过并报绿 tests/common/mod.rs:11-33
R8 L check_case 重算调用方已算过的 visible_counts(纯函数,无害) tests/...:387 vs :707
R9 审过 ATOL/RTOL = 5e-3 不掩盖真错误,且不该收紧(量化依据见 PR 正文容差节) tests/...:82-83;fixture 实测:一个块的网格平移中位数 1.49(p10 0.26)

修改措施

  • R1 已修(本轮新增提交 90fb0ff4):新增 device-free 测试 the_entry_point_refuses_a_geometry_it_was_not_compiled_for(tests/...:533),先跑两条零 extent 调用作正对照(编译期几何必须放行、且不形成 launch,否则下面的拒绝全部可能是"恒拒"的假通过),再逐条拒绝:640 两侧的 row stride、32 与 128 的 rotary 切分、1e6 与偏离 1e7 一个 f32 ulp 的 rope_theta、负与 NaN 的 rms_eps、三个负 extent、小于块网格的 score stride、以及输入/输出/工作区各一个空指针。所有调用都在第一次 launch 之前经 guard 返回,指针悬垂且从不解引用,故无需设备。验证:cargo test --release --locked -p pegainfer-kernels --features qwen4 --test qwen38fn_qsa_indexer_score_parity -- --nocapture → 3 passed。该 guard 不是给 ops wrapper 用的(wrapper 传自己的几何,永远触发不到),正是给直接调 FFI 的调用方用的——这也是它必须被测的理由。
  • R2 明确留给 CI 接线(就地更正:初稿把"本线不进 CI"归因给 feat(qwen4): detect the line and validate its geometry #1130,核对后 feat(qwen4): detect the line and validate its geometry #1130 是"detect the line and validate its geometry"那条 PR,与 CI 无关,已删除该归因):本 PR 不改共享 CI 配置——无法在本地验证一次 CI 运行,且矩阵是按模型线一行,加行属于该线的接入决定。PR 正文已写明"没有任何 CI job 启用 qwen4 feature(矩阵只在默认 feature 行带 -p pegainfer-kernels),故本 PR 的每处守卫在 CI 里都只是一个空 target、目前只在手跑命令下执行",并把"加行时须一并设 PEGAINFER_REQUIRE_GPU=1"(即 R7)写成同一条待办。
  • R3 保留,不改:这两条与 config 自己的 rotary_dim <= indexer_head_dim 校验(§2.7)同源,是"结构半边";精确几何 pin 是主判据,将来若为第二个 Qwen4 几何放宽 pin,它们才开始生效。删掉会让放宽后的结构校验一起消失。
  • R4 明确不修:判据工具选得对——max 判据负责抓结构性错误,相对判据负责抓尺度错误,两者合起来才是合适的量;均值偏置断言在这里几乎没有额外覆盖(独立翻转相互抵消,而尺度型错误会让残差符号随分数符号翻转、同样抵消),只会引入一个脆阈值把良性测量变成假阳性。诊断继续作为证据存在,并在 PR 正文容差节逐项引用(76/152 正、bias -5.2e-7、147/152 集中在一行)。
  • R5 已修:provenance 半边新增一行 println!,把 golden 记录的 transformers 5.16.1, torch 2.14.0, arm64 打出来(tests/...:207)。不改成断言:dump 主机只决定 float32 归约的舍入顺序,换一台主机同样是有效参考,锁死它会把正常重 dump 变成假失败。输出见 PR 正文证据块(两个测试各打一次)。
  • R6 保留,并已记录:fused_width/rotary_dim/rope_theta 由 entry point 在运行期拒绝(kHeads/kHeadDim 变化会改 kFusedWidth,同样被拒);只有 kRatio 没有任何运行期守卫,它的漂移会被门禁的块计数断言与 152/3076 的读数兜住——这一条已写进 PR 正文的 limitation。
  • R7 明确留给 CI 接线:与 R2 同一条,不重复开。
  • R8 保留,无害:visible_counts 是 fixture 上的纯函数,在 helper 内部重算让它自洽,不是第二种真相来源。
  • R9 结论:不收紧。六例 ≤9.537e-7(约 1–2 个 f32 ulp),七例合计 3674 个分数的最差为 bound 的 0.164;一个 bf16 输入 ulp 传导到分数的上界约 3e-3(3σ 元素),而既有标定(golden 跨主机重推导自身漂移 2.2e-3 / 0.11% 相对)已经在同一量级——再收紧到"一两个翻转"只会在别的卡/别的 torch 上产生假失败,买不到任何东西。反向证据:fixture 自测的一个块网格平移中位数 1.49,是 bound 的 17–100 倍,故门禁对结构性错误是响亮的。

@sysuTobo
sysuTobo marked this pull request as ready for review October 17, 2026 07:40
The scoring kernels move from csrc/qwen38fn/ to csrc/qwen4/ and take the
line's name, like qwen35 and glm52: the directory, the .cu, both kernel
entry points, the FFI symbol, the Rust modules and the parity test all
drop the checkpoint qualifier. The golden fixture keeps its qwen38fn
name — it is PR pegainfer-project#1150's committed data, recorded in its own provenance
manifest, and renaming it would churn every branch that replays it.

No semantic change: the same golden gate replays with the same numbers
(7 cases, worst bound fraction 0.164 of the 5e-3 bound).

Signed-off-by: bohaotu <bohaotu@pony.ai>
@sysuTobo
sysuTobo force-pushed the feat/qwen4-qsa-indexer-score branch from 513d01b to de9c111 Compare October 17, 2026 07:40
validate()'s first ensure pins rotary_dim == 64 and compress_ratio == 4
bit-for-bit, so the range checks behind it (rotary in 1..=head_dim,
compress_ratio > 0) can never fire — dead guards that read as extra
safety but guard nothing. The parity test's mutations are still caught
by the exact-equality pin, and it passes unchanged. The docs now also
say what the untouched score tail implies for a caller: telling an
unwritten entry from a real one means poisoning the buffer yourself,
which is what the gate's sentinel does.

Found in a data-flow audit of the PR; no behavior change.

Signed-off-by: bohaotu <bohaotu@pony.ai>
Condense the kernel, wrapper and gate comment blocks to what a reader
needs — the frozen math, the grid contract, the rounding points and the
bound's justification — and drop the internal planning labels
("Stage 1, plan P2", "decision D3-B") from the code. No semantic
change; the parity gate replays with the same numbers.

Signed-off-by: bohaotu <bohaotu@pony.ai>

@FeathBow FeathBow left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. On one sm_89 GPU the head commit builds with the qwen4 feature, clippy is clean and the gate prints exactly the output in the description. The fixture firmly pins the relu placement and the pooled key's RoPE anchor. Requesting changes because several statements in the description and the test comments do not hold when measured, and a reader would take them to mean the gate catches faults it lets through.

We changed one thing in the kernel at a time and ran the unchanged gate on the head commit.

Kernel change The description says Worst score vs the bound Gate
Relu after the head sum "17x to 100x the bound" 166x fails
Pooled key RoPE at the block end not listed 536x fails
Pool by sum instead of mean "17x to 100x the bound" 0.66x passes
rms_eps removed "pin every scalar constant" 0.66x passes
rms_eps times 100 "pin every scalar constant" 2.5x fails narrowly
Mean kept in f32 "17x to 100x the bound" 3.1x fails narrowly
cos and sin kept in f32 "Every bf16 rounding point of the reference is reproduced" 2.3x fails narrowly

The description also says the bound cannot be tightened because rsqrtf, cosf and sinf vary across architectures. Your sm_120 run and our sm_89 run printed identical output, and swapping the device trig for a host table built like pegainfer_core::rope leaves all 3,674 scores unchanged. The six short cases sit at 9.5e-7 while every passing or narrow row above moves them by 2.1e-3 or more, so holding cases 0 to 5 at that level and keeping 5e-3 only for the long case would turn all five into clear failures.

Before merge we would ask for four things.

  1. Correct the coverage and tolerance statements in the description, the test comment and the bring-up note to what was measured (inline).
  2. Tighten the short cases as above. The long case can keep its bound until its residual is traced.
  3. Make out of range rows or visible_len fail observably, and keep a NaN visible instead of scoring it zero (inline, both reproduced).
  4. Print the worst element's own coordinates next to max_abs (inline).

Smaller points.

  • The geometry plumbing and the budget guard callers that do not exist yet (inline).
  • The static_assert on kDimPerLane repeats its initializer, and the row_blocks > blocks clamp is implied by the visible <= tokens clamp above it.
  • The full_layers == 12 assertion is outside this kernel's contract, the final worst_scaled <= 1.0 assertion cannot fail because check_case panics first, and nothing reads the manifest assert_provenance returns.
  • The description still names the old qwen38fn paths and test target, so its commands do not run on this head.
  • Optionally, the kernel could read the cos and sin table that pegainfer_core::rope builds, since the reference indexer shares the position embeddings of attention. That drops kRopeTheta, rotary_dim and their checks, and on this fixture it left every score identical.

Comment on lines +44 to +63
/// Score parity bound, `abs(got - want) <= ATOL + RTOL * abs(want)`.
///
/// Six cases land within 9.6e-7 — every bf16 rounding decision the reference
/// makes is reproduced bit-for-bit. The residual sits in the seventh (the only
/// case with positions past 63): 152 of 3076 scores above 1e-4, worst 2.4e-3,
/// sign-balanced with a -5.2e-7 bias — the shape of a flipped bf16 rounding
/// decision, not a semantic difference. The reference rounds its float32
/// reductions in a torch-defined order; where a value sits within one f32 ulp
/// of a bf16 midpoint the kernel's butterfly tree rounds apart from it, and
/// one flipped element moves every score it touches. The reference does the
/// same to itself: re-deriving case 2 on x86 (torch 2.14.1 vs the golden's
/// 2.14.0/arm64) moves one of its 496 scores by 2.2e-3, three orders more than
/// this kernel's 9.5e-7 there.
///
/// So the bound holds ~6 flips at a typical score of ~2, against a measured
/// worst of 0.16 of it, while any real fault — a block off by one, a missing
/// relu, an unrotated half — moves a score by ~1, two orders out. Selection
/// must treat a block tied at the cut as an equally valid pick, for the same
/// reason.
const ATOL: f32 = 5.0e-3;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment and the matching note in the bring-up doc claim more than the gate shows. Pooling by sum and removing rms_eps both pass, dropping the mean's bf16 round trip fails at only 3.1x, and the reduction order account has no intermediate comparison behind it, so please replace the block shift estimate and that account with what was measured.

Comment on lines +334 to +337
/// One case's parity: how much was compared, the worst element and where, and
/// the shape of the residual — the sign split and the bias catch a
/// *systematic* error (a lean one way, deviations piling up in one row)
/// that a max alone cannot.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing asserts on the sign split, the bias or the widest row, so these fields cannot catch a systematic error as this comment says. Every score is nonnegative and RTOL is 5e-3, so a uniform scale error up to 0.5% passes every comparison with no sign to flip, and the sum pooling mutation shows the same balanced signs as the baseline. Please assert something these fields would catch, or drop them with the comment.

Comment on lines +441 to +450
max_abs = max_abs.max(abs);
signed_sum += f64::from(delta);
over_1e4 += usize::from(abs > 1.0e-4);
over_1e5 += usize::from(abs > 1.0e-5);
pos_1e4 += usize::from(abs > 1.0e-4 && delta > 0.0);
row_over_1e4 += usize::from(abs > 1.0e-4);
if scaled > max_scaled {
max_scaled = scaled;
case_worst = Some((block, *x, *want, abs));
worst = (*row, block, *x, *want);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

max_abs takes the largest absolute error while worst follows the largest scaled error, and the print presents them as one element. In the long case it prints max abs 2.401e-3 beside an element whose own difference is 2.255e-3. Please keep each metric's coordinates, or print only the metric they belong to.

Comment on lines +169 to +173
// `rows[row]` is the row's own position and the fused_qk row its query
// heads are read from; clamped, like `visible_len` below, so a broken
// caller gets a wrong position rather than an out-of-bounds read.
const int raw_pos = rows[row];
const int pos = raw_pos < 0 ? 0 : (raw_pos >= tokens ? tokens - 1 : raw_pos);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On our run rows = [8] and rows = [1000] against an 8 token buffer both return Ok with the scores of row 7, and rows = [-3] with visible_len = [-5] returns Ok and writes nothing. The repository already traps out of range device indices without a host copy (csr_page_row_checked and resident_row_checked in common.cuh, exercised by hd512_qk_rope_trap_page.rs), so the D2H cost the description cites does not apply. Please trap here as well and keep the valid empty prefix case successful.

Comment on lines +251 to +254
// relu per head, then the sum over heads, then the 1/sqrt(head_dim)
// scale: all three are the reference's own order.
const float relu_sum = fmaxf(acc[0], 0.0f) + fmaxf(acc[1], 0.0f) +
fmaxf(acc[2], 0.0f) + fmaxf(acc[3], 0.0f);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fmaxf returns the other operand when one is NaN, so a poisoned block scores exactly zero. With one raw key element of case 2 set to NaN, all 57 rows that see that block scored 0.0 where the clean reference has 2.62, 0.68 and 1.61, and the launch reported success. torch.relu keeps the NaN, so please let it through or fail instead of handing selection a valid looking lowest score.

Comment thread pegainfer-kernels/src/ops/qwen4.rs Outdated
Comment on lines +76 to +80
/// Refuse a geometry the kernel was not compiled for. Called by
/// [`qsa_indexer_block_scores_into`] before it forms a pointer, and public so
/// a caller deriving geometry from a checkpoint's config can check it
/// without a launch.
pub fn validate(&self) -> Result<()> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the literal from qwen38_flash_next() passes validate(), and the wrapper then hands the kernel the fused_width, rotary_dim and rope_theta it just pinned. The matching entry point checks can therefore fail only through the raw FFI, which is the path the refusal test exercises, while only_the_compiled_geometry_passes asserts the literals. Passing only rms_eps at run time, and checking the checkpoint's indexer fields where the model crate reads its config once that caller lands, removes the struct, both refusal layers and both tests.

Comment thread pegainfer-kernels/src/ops/qwen4.rs Outdated
Comment on lines +42 to +45
/// `indexer_budget`. Not read by this stage: the reference scores every
/// complete block and truncates at selection, so the budget only bounds
/// what comes next. Carried here so the caller passes one geometry around.
pub budget: usize,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The budget is unused by this stage and its divisibility check serves selection, so both can land with selection. The same holds for the page size reasoning in the max_blocks doc and for the gate's truncating and boundary assertions, which depend only on the fixture.

…rows

A mutation audit found the gate letting real semantic faults through: with
one 5e-3 bound everywhere, pooling by sum and dropping rms_eps both passed.
The pooled key's RMSNorm is scale-invariant, so those faults only move the
result through eps at the pooled key's scale — 3.3e-3 on this fixture — and
the long case's residual forced one bound on every case. Cases 0-5 sit at
<=9.6e-7 and now carry a 2e-6 bound; the long case keeps 5e-3 until its
residual is traced. The tight bound's floor is the measured mutation
minimum (3.3e-3 for the eps/pooling faults, 1.15e-2 and 1.55e-2 for the
missing bf16 round trips, 0.83 for a post-sum relu, 2.7 for a shifted RoPE
anchor), not architecture variance: sm_89 and sm_120 print identical
numbers and a host trig table leaves all 3,674 scores unchanged.

Correctness:
- Out-of-range `rows`/`visible_len` trap instead of clamping, following
  common.cuh's checked-row precedent; the valid empty prefix still
  succeeds. Two trap binaries, one per case, since __trap() is sticky.
- relu is a comparison rather than fmaxf, so a poisoned key stays NaN like
  torch.relu instead of scoring a valid-looking zero. New gate: one
  poisoned key makes exactly the rows that see its block score NaN, every
  other block finite, the untouched tail sentinel.
- max-abs and max-scaled each keep their own element coordinates; they
  were printed as one element.

Scope, from the same review:
- Geometry drops the unused budget and its divisibility guard (selection's
  business); the gate drops the fixture-only truncating/boundary asserts
  and the unreachable final bound assert.
- Removes the tautological static_assert, the dead row_blocks clamp, the
  out-of-contract full_layers assert, and assert_provenance's unused
  return.
- The bring-up note and gate comments state the measured numbers.

Signed-off-by: bohaotu <bohaotu@pony.ai>
The indexer shares the attention line's position embeddings, so the two
kernels now read the cos/sin table `pegainfer_core::rope` builds instead
of computing trig on the device. The entry point takes the two tables and
checks `rotary_dim` as their compiled row stride; `rope_theta` disappears
from the kernel, the FFI and the geometry, and the wrapper checks the
tables cover every position a call can read. The gate builds the same
table from the frozen config's own `rope_theta`.

Measured: every score is bit-identical to the device-trig version on all
seven cases (same max abs, coordinates and bias), so the parity numbers
are unchanged. 4 parity + 2 trap tests and 20 lib tests pass; fmt and
clippy are clean.

Signed-off-by: bohaotu <bohaotu@pony.ai>
…dant

Only the literal from `qwen38_flash_next()` could pass `validate()`, and the
wrapper then handed the kernel the same values, so the Rust validation and
the entry-point geometry checks could only fail through the raw FFI — the
path the device-free refusal test exercised. Following the review, the
struct, both layers and both tests are gone: the wrapper takes only
`rms_eps` at run time, sizes its buffer checks from the compiled constants,
and the checkpoint's indexer fields are checked where the model crate reads
its config once that caller lands.

The gate derives its geometry from the frozen config and cross-checks it
against the fixture's own tensor shapes instead of comparing it to a Rust
copy of the constants, so a config/geometry drift reaches the kernel as a
score mismatch rather than as a duplicated contract. The trap tests size
their buffers from local compiled-geometry constants.

No semantic change: parity 3/3 (provenance, comparison, NaN), traps 2/2,
lib 19, fmt and clippy clean.

Signed-off-by: bohaotu <bohaotu@pony.ai>
@sysuTobo
sysuTobo marked this pull request as draft October 17, 2026 17:19
@sysuTobo

sysuTobo commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor Author

Thanks @FeathBow — the mutation audit found real gaps, and the measured table is what changed this gate. All three rounds are on the head:

  • 9db501f0 — tight/loose bounds, device traps, NaN propagation, read-out coordinates, and the claims/asserts that measured false.
  • 7ee6b511 — your optional item: the kernel now reads the shared attention cos/sin table instead of computing trig.
  • 58ed7e05 — the geometry rail, removed as you suggested.

The four asks

1. Statements corrected. The description is rewritten (including the stale qwen38fn paths and test target, which is why its commands did not run on head), and the test comment / bring-up note now state the measured mutation floor in place of the block-shift estimate and the reduction-order account.

2. Short cases tightened. Cases 0–5 now carry ATOL = RTOL = 2e-6 (measured 8.9e-8 … 9.5e-7, a 5–11x margin); the 2055-token case keeps 5e-3 until its residual is traced. Under the new bound, pooling by sum and dropping rms_eps (3.3e-3 — the two that passed at 0.66x under the old bound; the pooled key's RMSNorm is scale-invariant, so pooling by sum differs only through eps at the pooled key's scale) now fail at ~600–1600x the bound, and mean-in-f32 and rms_eps×100 fail at 1.55e-2 / 1.25e-2. After the table change the cos/sin mutation is no longer expressible, so it is dropped from the floor.

3. Out-of-range metadata traps. rows and visible_len are checked in the kernel and trap, following common.cuh's csr_page_row_checked pattern — no D2H; the valid empty prefix (visible_len = 0) still succeeds and writes nothing. One trap per binary, per the sticky-context rule: qwen4_qsa_indexer_rows_trap.rs and qwen4_qsa_indexer_visible_trap.rs. For NaN, the relu is now a comparison rather than fmaxf, so a poisoned block keeps its NaN like torch.relu; the new gate poisons one key element and asserts exactly the 61 rows that see that block are NaN, every other block finite, and the untouched tail still sentinel.

4. Read-out coordinates. max abs and max bound fraction now each print their own element. On case 1 they are different elements (row 3 block 0 vs row 4 block 0), which is exactly the conflation you pointed at.

Smaller points

  • Geometry rail removed as suggested. QsaIndexerGeometry, its validate() and the entry-point fused_width/rotary_dim checks are gone — the wrapper now takes only rms_eps at run time and sizes its buffer checks from the compiled constants. Both refusal tests are gone with them. The gate derives its geometry from the frozen config and cross-checks it against the fixture's own tensor shapes, so a config/geometry drift reaches the kernel as a score mismatch instead of a Rust-side duplicate of the constants; the checkpoint's indexer fields get checked where the model crate reads its config, once that caller lands.
  • The unused budget and its divisibility guard are gone (selection's business), as are the fixture-only truncating/boundary assertions and the unreachable final worst_scaled assertion.
  • The dead row_blocks > blocks clamp is gone (the kernel lost the blocks argument with it), the static_assert now checks kHeadDim % WARP_SIZE instead of repeating its initializer, the out-of-contract full_layers assertion is gone, and assert_provenance's return is now consumed (the gate builds the table from the returned config's rope_theta).

Optional: the shared RoPE table (done)

The kernels take the two [max_pos, rotary_dim] tables and compute no trig. kRopeTheta and rope_theta disappear from the kernel, the FFI and the wrapper. Measured: all seven cases are bit-identical to the device-trig run — same max abs, same coordinates, same bias — which matches your host-table result. This also removes the architecture-variance argument entirely: there is no device trig left to vary.

Verification (head 58ed7e05)

  • parity gate: 3 passed (provenance, comparison, NaN); case 0 max abs 8.941e-8 … case 6 2.401e-3 at 0.164 of its bound
  • trap binaries: 1 passed each
  • cargo test -p pegainfer-kernels --features qwen4 --lib: 19 passed
  • cargo fmt --all -- --check clean; cargo clippy … --all-targets -D warnings clean

Still open and unchanged: the long case's residual is untraced, no CI row enables qwen4, and selection is the next stage.

@sysuTobo
sysuTobo marked this pull request as ready for review October 17, 2026 17:24
@sysuTobo
sysuTobo requested a review from FeathBow October 17, 2026 17:24
The trap tests only asserted when the call returned Ok, so any wrapper-side
Err — a drifted sizing constant, a new capacity check — would leave the gate
green without ever exercising the trap. The call's Ok is now the expected
path, and an Err is accepted only when it is the launch's own ("launch
failed"); a wrapper rejection fails the test. Both binaries still pass, so
the trap surfaces with no launch-side error on this box.

Signed-off-by: bohaotu <bohaotu@pony.ai>
@FeathBow

Copy link
Copy Markdown
Collaborator

@codex review please

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-17T21:15:32.894811Z aadacfd Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aadacfd44d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

/// `[max_pos, ROTARY_DIM]`, position-major): the indexer rotates with the same
/// position embeddings the attention uses, so this stage computes no trig. The
/// tables must cover every position this call can read, `0..tokens`.
pub fn qsa_indexer_block_scores_into(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Wire QSA scoring into a production request path

In the Qwen4 path I checked, pegainfer-qwen4 has no dependency on this kernel crate and pegainfer-qwen4/src/model_line.rs:29-30 still makes every launch call bail, so no production request can invoke this new operator. The golden and trap tests therefore validate only the isolated buffers and cannot exercise the projection/layout/stream integration required by the repository's production-E2E rule for features; either land this with the text-graph call site and a production E2E or defer the unreachable feature until that path exists.

AGENTS.md reference: AGENTS.md:L124-L126

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — the facts check out, and this is the one place this PR deliberately stops short.

  • pegainfer-qwen4/src/model_line.rs:28-30 does bail on launch; that is the line's shape since feat(qwen4): enforce the tensor contract before any device work #1149 — detection, config validation and the tensor contract, with no text graph yet.
  • Nothing production can reach this op today: it is behind pegainfer-kernels's additive qwen4 feature, which no production build enables (the server's qwen4 feature only turns on dep:pegainfer-qwen4, a CPU-only crate — pegainfer-server/Cargo.toml:48), and its only callers are the three test files.
  • The production call site needs the slice-B text graph: the index_qk_proj projection (2560 → 640), the stream and layout it feeds, and the scheduler step that consumes the scores.

So P2's deliverable is deliberately the component-level gate — golden parity, the trap contract and NaN propagation — and that boundary is written into the PR's "what is NOT verified" section rather than hidden. It is the same staging shape as this line's earlier component PRs (#1149, #1150, #1153), none of which has a production request path yet.

We will wire the op and carry a production E2E with the text-graph slice. If maintainers would rather not merge a component ahead of its caller, the alternative is to keep this PR draft and fold it into that slice — flagging so the merge-order call is explicit rather than accidental.

@github-actions github-actions Bot added the needs-rebase Conflicts with main, needs a rebase label Oct 20, 2026
@github-actions

Copy link
Copy Markdown

This pull request has merge conflicts with main. Please rebase it so it can be reviewed and merged.

…ct the gate notes

Signed-off-by: Feathbow <feathbow@gmail.com>
Signed-off-by: Feathbow <feathbow@gmail.com>
@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Commit attribution notice

This pull request contains commits attributed to an account other than the PR author @sysuTobo without a verified signature from that account:

If you are mentioned above and did not take part in this pull request, please say so here. Attributing a commit to someone else normally requires that person to be involved; maintainers should treat unacknowledged attribution as a review blocker.

This check never fails the build.

@github-actions github-actions Bot removed the needs-rebase Conflicts with main, needs a rebase label Oct 20, 2026
… stale notes

Signed-off-by: Feathbow <feathbow@gmail.com>
@FeathBow

Copy link
Copy Markdown
Collaborator

We pushed three commits to this branch on top of your four, two fixes and a merge of main.

Your four commits close the four points of the earlier review. The traps, the NaN path, the tight bound and the separate read-out coordinates all hold up. We checked each one by breaking the kernel on one sm_89 GPU with the gate unchanged. The numbers are how far the first failing score is from the reference.

Kernel change At the first review At your latest push, before our commits
pool by sum passes fails, 8.1e-4 off
rms_eps dropped passes fails, 3.1e-4 off
rms_eps × 100 fails fails, 3.9e-4 off
pooled mean kept in f32 fails fails, 2.5e-3 off
pooled key rotated at the block end fails fails, 0.90 off
relu after the head sum fails fails, 0.31 off
query rotated with a neighbouring table row fails, 0.096 off
relu written as fmaxf the NaN scored 0 and passed the NaN gate fails
rows or visible trap removed the bad index was clamped and passed the trap test fails

What our commits change.

  • A visible_len past the row's own token was accepted. Row 3 of an 8 token buffer with visible 8 returned two scores, the second one for tokens 4 to 7. The kernel now traps when visible_len[j] > rows[j] + 1. Since rows[j] is already below tokens, that one check also covers the array bound. The visible trap test now uses that case, which a check against the array length alone lets through.
  • The trap tests accepted any launch error, because the wrapper puts "launch failed" in front of every error the entry point returns. They now require CUDA_ERROR_LAUNCH_FAILED, the error a device trap ends in, and both share one launch helper in tests/qwen4_qsa_trap/.
  • The gate's table matches RopeTableSpec::tables only when frequency_dim equals rotary_dim. That is how the reference spaces the frequencies of its 64 rotated dims, and the gate's agreement with the golden confirms it. The wrapper doc and the test comment now say so.
  • Notes that no longer matched the code are fixed. The FFI header no longer says the entry point refuses a mismatched rotary geometry. The provenance comment gives the reduction order as one candidate for the long case's residual. The trap headers say that no CI job enables qwen4, so CI neither builds nor runs them. The budget divisibility assertion is gone, since only selection depends on it.
  • bring-up.md gives the long case's cause as a hypothesis until it is traced, and the trap sentence includes the cap at the row's own token. The file now keeps each paragraph on one line, so most of its diff is line joins with the wording unchanged.
  • feat(qwen4): share the Qwen3.5 GDN decode wrappers and prove their reuse #1159 landed and also edits bring-up.md. The merge takes main's Config paragraph, which describes the shared GDN wrappers, and keeps this PR's QSA section and its note under Traps.

What our trap changes catch, checked the same way.

Change At your latest push Now
entry point returns CUDA_ERROR_INVALID_VALUE before any launch both trap tests pass both fail, they report the invalid value
visible cap at the array length the visible trap test passes, it only covered the array bound the visible trap test fails
rows trap removed the rows trap test fails the rows trap test fails

The description still refers to code this head no longer has. The gate has one device free test, fixture_records_the_frozen_reference, and there is no ops unit test. QsaIndexerGeometry and the entry point test for tokens < 4 are gone. The scores are sums of relus and never negative, so a scale error keeps one sign across the residual and does not cancel in the bias.

Verification at the new head on one sm_89 GPU, with main's FlashInfer pin.

Check Result
cargo fmt --check, and clippy with -D warnings for pegainfer-kernels --features qwen4,qwen35 clean
qwen4_qsa_indexer_score_parity 3 passed, output unchanged
qwen4_qsa_indexer_rows_trap and qwen4_qsa_indexer_visible_trap 1 passed each

@FeathBow
FeathBow merged commit 1f17ddb into pegainfer-project:main Oct 20, 2026
14 checks passed
sysuTobo added a commit to sysuTobo/pegainfer that referenced this pull request Oct 21, 2026
The Qwen3.8-Flash-Next MoE golden had a device-free consumer only, so it
pinned a contract without executing any production code. Add its device
half inside pegainfer-kernels, behind the line's additive `qwen4` feature, appending
to the module pegainfer-project#1160 opened, so the model crate stays CUDA-free and the
feature keeps the new translation unit out of default builds:

  csrc/qwen4/qwen4_moe_route.cu         routing + shared-expert combine
  src/ffi/qwen4.rs                      the two C entry points, appended
  src/ops/qwen4.rs                      geometry/semantics guards, wrappers
  tests/qwen4_moe_block_parity.rs       the gate, through the ops layer

Routing is fp32 softmax over 512 -> top-10 in the canonical order
(descending probability, ascending index inside a tie) -> optional
`norm_topk_prob` renorm summing in that same order -> bf16 weights and
i32 ids, one block per token over fixed shuffle trees so the picks are
deterministic run to run; a non-finite logit fails its row closed by a
block-uniform vote before the softmax, so no NaN reaches the arithmetic.
The combine folds the learned per-token sigmoid gate into the block sum,
rounding where the reference rounds. Everything else is composition: the
router GEMM, the shared expert's three projections and its SwiGLU go
through `gemm_into_checked` / `silu_mul_batch_into`, so the gate grades
the production path.

Nothing in-tree carries this routing contract, which is why the kernel is
new: gemma4's router is 128 experts with `__expf`, an unconditional
renorm and f32 weights; glm52 (256/top-8) and kimi (384/top-8) score with
sigmoid + bias; FlashInfer's FilteredTopK does no softmax at all. The
combine is the one near-duplicate -- `add_scaled_bf16_kernel`'s
arithmetic, which takes its scale as one host f32 and so cannot carry a
per-token device gate -- and the doc names it as such.

Measured on an RTX 5060 (sm_120), CUDA 12.9.1, every bound derived before
it was checked: routing bitwise 210/210 on the stored logits, the chain
0.0397 relative against the derived 0.1332, the combine bitwise
53760/53760 on stored operands and <=0.371 of its bound on the device
branch, and three controls that are not vacuous thresholds (no-renorm
210/210 at 146x, bf16 softmax 56/210 with no re-ordered picks, no shared
gate 76% of outputs at 125x). The device-free replay is untouched and
still green.

Signed-off-by: bohaotu <bohaotu@pony.ai>
sysuTobo added a commit to sysuTobo/pegainfer that referenced this pull request Oct 23, 2026
One indexer, one golden gate: scoring and selection now live in
`qwen4_qsa_indexer_parity`, sharing the fixture path, provenance check and
tensor readers instead of duplicating them across two files. All five tests
and every assertion are preserved; the select half still replays the
fixture's recorded scores.

The five trap binaries keep one trap per process, because `__trap()` leaves
the context poisoned. Their bodies move into the shared `qwen4_qsa_trap`
launcher, leaving each binary about eighteen lines like pegainfer-project#1160's, and the
device-reset warning is recorded once in the launcher.

Signed-off-by: bohaotu <bohaotu@pony.ai>
sysuTobo added a commit to sysuTobo/pegainfer that referenced this pull request Oct 23, 2026
One indexer, one golden gate: scoring and selection now live in
`qwen4_qsa_indexer_parity`, sharing the fixture path, provenance check and
tensor readers instead of duplicating them across two files. All five tests
and every assertion are preserved; the select half still replays the
fixture's recorded scores.

The five trap binaries keep one trap per process, because `__trap()` leaves
the context poisoned. Their bodies move into the shared `qwen4_qsa_trap`
launcher, leaving each binary about eighteen lines like pegainfer-project#1160's, and the
device-reset warning is recorded once in the launcher.

Signed-off-by: bohaotu <bohaotu@pony.ai>
sysuTobo added a commit to sysuTobo/pegainfer that referenced this pull request Oct 24, 2026
The kernel, its C ABI, the ops surface and six exported constants carried
a gate-only mutation parameter. Measure the five controls with temporary
kernel patches instead, as the pegainfer-project#1160 gate did, and keep the production
launch free of the knob. The golden, tie and structural assertions stay;
the fixture still covers both selection paths.

Signed-off-by: bohaotu <bohaotu@pony.ai>
FeathBow added a commit that referenced this pull request Nov 1, 2026
…1178)

* feat(qwen4): select QSA indexer blocks and their tail in native CUDA

Signed-off-by: bohaotu <bohaotu@pony.ai>

* refactor(qwen4): keep the selector's gate knob out of the production entry

The only public entry took the gate's mutation variant as a required
argument, so a production caller had to spell a value that must always be
0 and could silently select a negative-control path. Split it the way the
PLE block does: `qsa_indexer_select_into` fixes the production variant,
and `qsa_indexer_select_variant_into` carries the knob for the parity
gate. The kernel is unchanged.

Also record the trap binaries' device side effect: on an RTX 5060 under
CUDA 12.9.1 a deliberate `__trap()` left the whole GPU uninitializable
for later processes, so they must run last or on a disposable device.

Signed-off-by: bohaotu <bohaotu@pony.ai>

* fix(qwen4): tighten the QSA selector's guards and docs

- Treat an empty launch as a no-op in the selector, matching the score
  stage and the other kernels' convention; the C entry still refuses one
  reached directly.
- Count the selector's static shared state against the 48 KiB per-block
  ceiling so an exact-boundary launch cannot be refused by the driver.
- Drop the dead int64 overflow guard: an int x int product always fits.
- Correct the budget error text and the module/feature docs, which still
  described selection as a later stage.
- Trap a negative `rows` entry in its own binary; the negative half of the
  row-position guard was untested.
- Record in the bring-up doc that ties at the cut are not an edge case at
  long context, so an end-to-end accuracy gate must not require mask
  equality with the reference there.

Signed-off-by: bohaotu <bohaotu@pony.ai>

* test(qwen4): merge the indexer gates and share the trap launcher

One indexer, one golden gate: scoring and selection now live in
`qwen4_qsa_indexer_parity`, sharing the fixture path, provenance check and
tensor readers instead of duplicating them across two files. All five tests
and every assertion are preserved; the select half still replays the
fixture's recorded scores.

The five trap binaries keep one trap per process, because `__trap()` leaves
the context poisoned. Their bodies move into the shared `qwen4_qsa_trap`
launcher, leaving each binary about eighteen lines like #1160's, and the
device-reset warning is recorded once in the launcher.

Signed-off-by: bohaotu <bohaotu@pony.ai>

* test(qwen4): run every indexer trap from one child-process gate

The five trap binaries each carried a case, but the cases are one line of
setup. Keep one trap per process — `__trap()` poisons the context — and let
one gate re-execute itself with `QSA_TRAP_CASE`, so the indexer has two
test files: the golden parity gate and this trap gate. The child sets
`PEGAINFER_REQUIRE_GPU=1` so a missing device fails instead of reporting a
trap it never reached.

Signed-off-by: bohaotu <bohaotu@pony.ai>

* style(qwen4): keep the qwen4 re-exports rustfmt-ordered

Signed-off-by: bohaotu <bohaotu@pony.ai>

* fix(qwen4): take the selector's score stride explicitly

The selector inferred `score_cols` from `scores.len() / rows.len()`. The
score stage allows a buffer with spare capacity, so handing the selector
that buffer produced a larger stride, passed both host checks and read
every row from the wrong offset. Take `score_cols` like the score stage
does, check `scores.len() >= rows * score_cols`, and cover the spare-
capacity case in the launcher tests.

Signed-off-by: bohaotu <bohaotu@pony.ai>

* refactor(qwen4): drop the selector's gate-only variant knob

The kernel, its C ABI, the ops surface and six exported constants carried
a gate-only mutation parameter. Measure the five controls with temporary
kernel patches instead, as the #1160 gate did, and keep the production
launch free of the knob. The golden, tie and structural assertions stay;
the fixture still covers both selection paths.

Signed-off-by: bohaotu <bohaotu@pony.ai>

* perf(qwen4): skip the selection search when every block is kept

Rows under the budget keep every complete block, so the mask is the whole
visible prefix. Skip the masked argmax loop and its barriers for them; the
parity gate already counts those rows and asserts the fixture covers both
paths.

Signed-off-by: bohaotu <bohaotu@pony.ai>

* fix(qwen4): fail the trap gate when its child runs no test

A filter that matches nothing (`--exact` after a rename) exits 0 with
`0 passed`, so checking only the exit status let every case pass without
reaching a trap. Require the child to report one passed test.

Signed-off-by: bohaotu <bohaotu@pony.ai>

* perf(qwen4): radix-select the indexer's top blocks

The masked-argmax selection cost one full row scan and two barriers per
kept block: 86 ms for a single 256k-token row, 18 ms at 64k and 0.9 ms at
the first truncating row. Replace it with an exact 8-bit radix select:
four histogram passes find the keep-th sortable key, then the blocks
equal to the threshold are filled in ascending block-index order (the
canonical tie rule). The all-kept fast path stays.

Measured on one L20, one row, budget 2048, nsys kernel medians at 256
threads: 20.8 us at 2052 tokens, 34.4 us at 8k, 67.0 us at 64k, 298 us
at 256k, and 1.4 us for the all-kept fast path. The parity gate passes
5/5 and compute-sanitizer memcheck/racecheck are clean. Add the manual
benchmark example used for those numbers.

Signed-off-by: bohaotu <bohaotu@pony.ai>

* docs(qwen4): label the selector thread-sweep timing protocols

Signed-off-by: bohaotu <bohaotu@pony.ai>

* fix(qwen4): keep the benchmark's long literals readable

Clippy's unreadable_literal gate rejects 65535 and 262143 without
separators; use separators so the manual benchmark stays clean under
-D warnings.

Signed-off-by: bohaotu <bohaotu@pony.ai>

* refactor(qwen4): keep the branch to the selector and its parity gate

The score stage's test files, parity gate and trap binaries are restored
byte-for-byte from main; selection tests sit beside the score test in
`qwen4_qsa_indexer_score_parity.rs` without moving the score half.

The select kernel header is the one place for the contract; the C entry and
the Rust wrapper keep only what each side owns. The `SelectPlan` split and
its five device-free tests fold back into the wrapper, the benchmark example
and its Cargo entry go, and the bring-up doc keeps only what the code cannot
say: the radix select's 256k result, the token-0 grid, and the tied-cut rule.

Signed-off-by: bohaotu <bohaotu@pony.ai>

* docs(qwen4): bump the bring-up freshness stamp

Signed-off-by: bohaotu <bohaotu@pony.ai>

* refactor(qwen4): trim the QSA selector's unreachable checks and repeated docs

Signed-off-by: Feathbow <feathbow@gmail.com>

---------

Signed-off-by: bohaotu <bohaotu@pony.ai>
Signed-off-by: Feathbow <feathbow@gmail.com>
Co-authored-by: Feathbow <feathbow@gmail.com>
sysuTobo added a commit to sysuTobo/pegainfer that referenced this pull request Nov 1, 2026
The Qwen3.8-Flash-Next MoE golden had a device-free consumer only, so it
pinned a contract without executing any production code. Add its device
half inside pegainfer-kernels, behind the line's additive `qwen4` feature, appending
to the module pegainfer-project#1160 opened, so the model crate stays CUDA-free and the
feature keeps the new translation unit out of default builds:

  csrc/qwen4/qwen4_moe_route.cu         routing + shared-expert combine
  src/ffi/qwen4.rs                      the two C entry points, appended
  src/ops/qwen4.rs                      geometry/semantics guards, wrappers
  tests/qwen4_moe_block_parity.rs       the gate, through the ops layer

Routing is fp32 softmax over 512 -> top-10 in the canonical order
(descending probability, ascending index inside a tie) -> optional
`norm_topk_prob` renorm summing in that same order -> bf16 weights and
i32 ids, one block per token over fixed shuffle trees so the picks are
deterministic run to run; a non-finite logit fails its row closed by a
block-uniform vote before the softmax, so no NaN reaches the arithmetic.
The combine folds the learned per-token sigmoid gate into the block sum,
rounding where the reference rounds. Everything else is composition: the
router GEMM, the shared expert's three projections and its SwiGLU go
through `gemm_into_checked` / `silu_mul_batch_into`, so the gate grades
the production path.

Nothing in-tree carries this routing contract, which is why the kernel is
new: gemma4's router is 128 experts with `__expf`, an unconditional
renorm and f32 weights; glm52 (256/top-8) and kimi (384/top-8) score with
sigmoid + bias; FlashInfer's FilteredTopK does no softmax at all. The
combine is the one near-duplicate -- `add_scaled_bf16_kernel`'s
arithmetic, which takes its scale as one host f32 and so cannot carry a
per-token device gate -- and the doc names it as such.

Measured on an RTX 5060 (sm_120), CUDA 12.9.1, every bound derived before
it was checked: routing bitwise 210/210 on the stored logits, the chain
0.0397 relative against the derived 0.1332, the combine bitwise
53760/53760 on stored operands and <=0.371 of its bound on the device
branch, and three controls that are not vacuous thresholds (no-renorm
210/210 at 146x, bf16 softmax 56/210 with no re-ordered picks, no shared
gate 76% of outputs at 125x). The device-free replay is untouched and
still green.

Signed-off-by: bohaotu <bohaotu@pony.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request qwen4 Qwen4-architecture model crate (pegainfer-qwen4)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants