Repository navigation
Run an affine qmv_wide verify of 6 or 7 rows as one tile (gemma4-qat #191) - #9
Conversation
affine qmv_wide tiled at most 5 input vectors per weight pass, so a speculative verify of 6 or 7 rows (gemma4-qat k = 6 verifies M = 7) ran two tiles and streamed every weight twice. affine M = 6..7 is now one tile. A vector's accumulation does not depend on the tile, so the output bits match the old split. The implied-bias (_ib) path is the same templated kernel and needs no kernel change. Every other M and mode keeps the cap of 5 byte for byte: M = 8 stays 4 + 4 and M = 11/12 stay three tiles. On M3 Max a tile of 8 costs 10-16% more than a tile of 7, which made M = 8 break-even or worse against 4 + 4; MLX's dense gemv_wide also caps at 5 for occupancy. affine qmv_wide runs only on gen-15+ (use_qmv_wide); the fp modes run on every generation and are unchanged. The non-JIT library instantiates affine_qmv_wide and its _ib variant for tiles 6 and 7. A C++ doctest and a Python test check that M rows in one call match the same rows in chunks of at most 5, with explicit biases and the implied-bias factor.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c74cbf19da
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // affine (gen-15+ only, see use_qmv_wide) runs M = 6..7 as one tile, so a | ||
| // speculative verify of up to 7 rows streams the weights once; a vector's | ||
| // accumulation does not depend on the tile, so the bits match the split. | ||
| // Every other M and mode keeps the cap of 5. M3 Max: a tile of 8 costs | ||
| // 10-16% more than a tile of 7 (MLX's dense gemv_wide caps at 5 for | ||
| // occupancy), so M = 8 stays 4 + 4. |
There was a problem hiding this comment.
Reduce this six-line comment to the essential invariant. The code already shows the affine limit and the surrounding function explains tile selection, while the speculative-decoding and benchmark details add excessive inline commentary that the repository standards explicitly prohibit.
AGENTS.md reference: AGENTS.md:L29-L33
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d23d9b96d8
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # affine qmv_wide runs M = 6..7 as one tile; every other M keeps the | ||
| # cap of 5 (8 is 4 + 4, 11 is 4 + 4 + 3, 12 is 4 + 4 + 4). A vector's | ||
| # accumulation does not depend on the tile, so one call over M rows | ||
| # must match, bit for bit, the same rows run in chunks of at most 5 | ||
| # (one tile each); the tiling itself is not observable from here. All |
There was a problem hiding this comment.
Reduce this seven-line block to the essential assertion invariant. It repeats the split values defined immediately below and includes internal routing details that the test cannot observe, contrary to the repository requirement to keep comments concise and avoid redundant inline commentary.
AGENTS.md reference: AGENTS.md:L31-L32
Useful? React with 👍 / 👎.
| // affine qmv_wide runs M = 6..7 as one tile; M = 8 keeps main's 4 + 4. A | ||
| // vector's accumulation does not depend on the tile, so each call must match, | ||
| // bit for bit, the same rows run in chunks of at most 5 (one tile each), and | ||
| // stay close to an fp32 dequantize-then-matmul reference. The tiling itself is | ||
| // not observable from here. Explicit biases and the 0-d implied-bias factor. |
There was a problem hiding this comment.
Reduce this five-line comment to the essential test invariant. The test name, split table, bitwise comparison, and reference calculation already show these details, so the block adds redundant commentary beyond the repository's usual one-to-two-line limit.
AGENTS.md reference: AGENTS.md:L31-L32
Useful? React with 👍 / 👎.
1 participant
Part of numen-tech/gemma4-qat#191 (epic numen-tech/gemma4-qat#178). Fork only; no upstream PR.
What
qmv_wide(quantized.cpp) runs an affine call of M = 6 or 7 input vectors as one tile (vecs_per_tg = M). Main's cap of 5 split them 3 + 3 and 4 + 3, and each tile re-reads every weight.A speculative verify with k drafts runs M = k + 1 rows. The shipped k = 6 verifies M = 7, so after this change it streams the weights once instead of twice.
gemv_widecap at 5.qmv_wideonly runs on GPU gen ≥ 15 (use_qmv_wide). For K, N in the shipped range, gen-15+get_qmv_batch_limitis ≥ 12, so M = 6/7 stays onqmv_wide.affine_qmv_wide(plain and_ib, batched and not) for tile sizes 6 and 7.affine_qmv_widevariants in the metallib. Trimming them to the shipped widths would need a second host-side predicate, so I left them all in.Numerics
A vector's accumulation does not depend on which tile it sits in, so outputs are bit-identical to the old split. End to end at k = 6, greedy output is byte-identical on E2B, E4B and the 12B.
Verification
test_quantized.py: 56 passed, 7,900 subtests./simplifypass: folded the doctest's per-group random draws into a lambda.The tok/s and verify-cost evidence is on the gemma4-qat PR.
Merge order
Source/Cmlx/mlxat the mergedprism-0.32.2-fixescommit and regenerate;Package.resolved.