moe: bound W4A16 scratch and repair tail correctness - #100
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Rental-machine validation — repaired composition now PASSInitial tested head: Host: Vast.ai 46335896; 4x RTX PRO 6000 Blackwell, all Initial result — isolated PASS, composition FAIL
That failure was real and promotion was blocked. The four defects found during the repair campaign were inherited W4A16 correctness/memory-safety bugs, not bugs introduced by the planner PR. Initial commit Current exact-head result — PASS
The planner saving remains 64.90904 MiB/rank. A matched three-run 600 W A/B on aligned GLM-like I=1856 shapes found byte-identical output/allocation/peak-memory and latency deltas from -0.084% to +0.068%; no throughput claim is made for the safety repair. Full chronology, exact commands, raw XML/logs, sanitizer evidence, and A/B JSON: repair ledger at Exact r14 deployable-source replayThe immutable r14 SparkInfer release tree was independently reconstructed from On physical GPU 2 with a fresh candidate-specific extension cache, the complete seven-file union passed 332 passed / 16 intentional skips cold in 237.03 s and identically warm in 10.03 s. This closes the image-source composition boundary for the W4 repair; no new numerical, compile-cache, capture, or mixed-K regression appeared. Exact source provenance and immutable raw logs: r14 reconstruction ledger, cold log, warm log. |
|
Final cross-stack evidence for exact head c2f135c: the byte-exact planner query confirms 68,062,064 bytes, or 64.90904 MiB/rank, of unreachable W4A16 reserve removed. The repaired #100 plus #102 composition passed 88/88 boundary/poison tests, 24/24 Compute Sanitizer cases with zero errors, and 220 passed/16 skipped cold and warm; matched production-shape latency was neutral within noise. This 64.909 MiB is an independent sizing claim, not yet a measured combined full-model delta with vLLM #210. Final evidence: https://github.com/malaiwah/glm52-exl3-vast/blob/5c76a2536e7fc9a5f1cb6bf182531889f5385e65/docs/field-review-results/2026-07-30-vast-46335896/UPSTREAM-REPAIR-CAMPAIGN.md Reproduction: https://github.com/malaiwah/glm52-exl3-vast/blob/5c76a2536e7fc9a5f1cb6bf182531889f5385e65/docs/field-review-results/2026-07-30-vast-46335896/COUNTER-VALIDATION.md |
Review-base noteThis PR still targets I am therefore leaving the tested head/base untouched instead of manufacturing a misleading clean check. Reviewers should use exact head |
Summary
Bound unpinned W4A16 scratch planning to route block sizes reachable within the declared token capacity, mirror the correction in replicated-input EP, and expose a cheap public arena-sizing query.
This is stacked on #92 because the fixed-capacity Trellis planning API under review there is the source base. It addresses the safe planner work from #93; it does not clamp Trellis capacity or implement caller-side prefill chunking.
Related: #93
Problem
fused_moeandep_moesized unpinned W4A16 route buffers and both fp32c_tmpplanes across every allowed block size(8, 16, 32, 48, 64), even when runtime selection cannot reach most of them withinmax_tokens.At
max_tokens=64,topk=8,E=160, SM count 188, hidden 6144, intermediate/rank 512, the current full sweep prices the non-full W4A16 arena at 94.94 MiB. Runtime can select only block 8, whose exact arena is 30.03 MiB: 64.91 MiB/rank of unreachable reserve.The review also exposed two sizing hazards that a naive narrowing would make visible:
m * topk * block_mscratch geometry instead of packed-route geometry;weight_E, even when the router workspace uses a smallerroute_num_experts.Both are covered explicitly rather than relying on the old oversized sweep.
Change
route_block_sizes_for_capacity(max_tokens, topk, num_experts)using the same monotone selector as runtime.c_tmpindependently for both packed-route and every reachable direct-topk/TC-decode(m, block)geometry.weight_Efor every non-Trellis W4A16 core route allocation; retain requestedroute_Efor mapped full-rotation Trellis.fused_moe.required_nbytes(Caps), which returns the complete caller-owned arena requirement without launch compilation or retained storage.Safety invariants
1 <= m <= max_tokensis included.c_tmp; packed route-index and block-expert buffers are not inflated by direct geometry.expert_offsetsand route buffers match the expert count the runtime actually packs.Additional correctness value found during GPU qualification
The 64.91 MiB/rank planner reduction is one value axis. GPU qualification also found four pre-existing W4A16 correctness/memory-safety defects that are worth landing independently of how reviewers value that memory saving. They were not introduced by the planner patch: initial commit
fa7a6adchanged host-side planning/allocation, while the affectedmicro.pyandprepare.pyimplementation was unchanged from base06032ce.The smaller, correctly bounded arena made the first latent lifetime violation reproducible; poisoned replay, boundary, and Compute Sanitizer tests then exposed the remaining inherited defects:
The current GLM
I=1856production shape is aligned and does not exercise those tail cases; matched A/B is therefore byte-identical and throughput-neutral there. The generic fixes still remove real wrong-output/OOB behavior for supported W4A16 shapes. In short: the planner change recovers memory, while the later commits make the inherited kernel implementation safe and correct.Observed sizing
CPU-isolated query against the r12 image dependencies and this source:
The shipped vLLM EXL3 integration pins both decode and prefill block sizes, so the 64.91 MiB correction does not change that deployment directly. The new sizing query is the prerequisite for pricing the separate caller-side prefill-capacity tradeoff.
Verification
All final validation ran in a separate CPU-only container (
--runtime crun,NVIDIA_VISIBLE_DEVICES=void, emptyCUDA_VISIBLE_DEVICES) using the exact pulled r12 image dependencies:Covered suites:
test_fused_moe_planning.pytest_fused_moe_trellis.pytest_fused_moe.pytest_ep_moe_api.pytest_w4a16_route_pack.pyChanged-file Ruff lint and
git diff --checkpass.Independent review initially found the direct-topk and non-Trellis expert-count hazards above. Regression tests were added for both exact cases; after both corrections the final verdict was APPROVE, with no remaining actionable defect. Residual risk is live-GPU execution, which is intentionally not claimed for this host-side planner-only change while the production deployment remains active.