Skip to content

test: gemv_q4 oracle uses f16 scales (match the kernel) - #27

Merged
TheTom merged 3 commits into
devfrom
tom/fix/gemv-q4-test-f16-scales
Jul 18, 2026
Merged

test: gemv_q4 oracle uses f16 scales (match the kernel)#27
TheTom merged 3 commits into
devfrom
tom/fix/gemv-q4-test-f16-scales

Conversation

@TheTom

@TheTom TheTom commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

What

mt_gemv_q4_coalesced reads f16 scales (the resident-weight decode/prefill path uploads them as f16). gemv_q4_matches_cpu_dequant was uploading quantize_q4's f32 scales and referencing against f32 — the wrong dtype. That mismatch (f32 scales → f16 kernel → garbage) is what motivated ffai-kernels#33's wrong "fix" (flipping the kernel to f32), which then broke the real f16 decode/prefill with NaN.

This uploads the scales as f16 and references against the f16-rounded value, so the kernel and CPU oracle agree exactly.

Validation

Passes on Metal against the f16-reverted kernel (ffai-kernels#36).

Coordination

Pairs with ffai-kernels#36 (reverts mt_gemv_q4_coalesced/_relu2/_accum to f16). Merge #36 first; until then this PR's CI runs against the f32 kernel on ffai-kernels dev and the test fails by design (re-runs green once #36 lands).

mt_gemv_q4_coalesced reads f16 scales (resident-weight decode/prefill feed
f16). gemv_q4_matches_cpu_dequant was uploading the quantize_q4 f32 scales
and referencing against f32 — feeding the wrong dtype and motivating the
ffai-kernels#33 regression. Upload the scales as f16 and reference against
the f16-rounded value so the kernel and CPU oracle agree exactly.

Pairs with ffai-kernels#36 (reverts the kernel to f16). Merge that first.
@github-actions github-actions Bot added the test Tests only — no behavior change label Jun 25, 2026
@TheTom

TheTom commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

Status: the test fix itself is still correct (q4 kernel on current kernels dev reads f16 scales — the doc comment there now explicitly warns against flipping it back). But this PR can't go green on its own: the whole rust/ tree has drifted against ffai-kernels dev since June. The Rust CI job fails at MetalTileError (renamed to FFAIError in #40/#41), and fixing that surfaces ~52 more rename errors, and fixing those surfaces numeric failures — kernel signatures evolved (e.g. ffai_gemv_q4_coalesced grew rows_per_group/rows_per_tg multi-warp constexprs + a generic activation dtype) while the rust dispatch sites still use June geometry, so gemv q4/q8 return garbage.

Groundwork parked on tom/fix/rust-kernels-dev-resync (mt_→ffai_ sweep + Cargo.lock repin to 71575250; compiles clean, numerics still red). Remaining work: audit every ffai-ops dispatch against current gemv/ssm/scan/sdpa signatures. This PR should merge after (or as part of) that resync — not before.

The Cargo.lock pinned pre-rename kernels (d4cafc60), so the tree still
compiled against MetalTileError and mt_-prefixed kernel fns; against
current dev the Rust CI job fails at the first unresolved import.

- Rename sweep: mt_ -> ffai_ identifiers and kernel-name references
  across rust/crates (7 files), matching kernels #40/#41.
- Repin the six ffai-kernels crates to dev 9fe86104.

Audited all ffai-ops dispatch sites against current kernel definitions
(param order, constexprs, grid/TG geometry, dispatch-invariant docs) —
no geometry drift; the intermittent local gemv failures are the known
parallel PSO-compile race the Rust CI job already pins to one test
thread. cargo check --workspace --all-targets clean;
cargo test -p ffai-metal -p ffai-ops (single-thread, on GPU): 47
passed, 0 failed, 1 ignored.
@TheTom

TheTom commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

CI unblocked in 5765305: the rust/ tree was pinned to pre-rename kernels (d4cafc60) — resynced with the mt_→ffai_ sweep and a repin to dev 9fe86104. Full dispatch-site audit found no geometry drift (my earlier gemv-signature suspicion was wrong — the rust caller already passes rows_per_group/rows_per_tg); the intermittent local gemv failures were the parallel PSO-compile race the Rust CI job already pins to one thread. On-GPU suite: 47 passed / 0 failed single-threaded, including this PR's q4/q8 oracle tests. Merging on green.

@TheTom
TheTom merged commit edfc0ce into dev Jul 18, 2026
6 of 7 checks passed
@TheTom
TheTom deleted the tom/fix/gemv-q4-test-f16-scales branch July 18, 2026 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Tests only — no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant