Normalize W4A16 profiler scope grammar and report unique expert count - #1074
Draft
roberteg16 wants to merge 1 commit into
Draft
Normalize W4A16 profiler scope grammar and report unique expert count#1074roberteg16 wants to merge 1 commit into
roberteg16 wants to merge 1 commit into
Conversation
#1067 made the W4A16 labels self-describing but left three spellings of the same field in circulation: bare `g32` on the dense scopes, `g=32` on the MoE ones, and `gs=`/`sk=` on the AWQ GEMVs. Settle on `key=value` everywhere so a label consumer needs one grammar rather than three. The MoE scope also could not be turned into a bandwidth number. Weight slabs dominate its traffic and the kernel loads one per block, but a trace records expert_ids' length, not its contents -- and several blocks can share an expert, so the block count is only an upper bound (E and top_k give a looser one still). Count the distinct ids and emit them as `experts=`. Reading that count forces a device->host sync, so it is guarded exactly as _moe_gemm_w4a16_scope already guards its own: the helper returns a nullcontext before touching the tensor unless profiling scopes are enabled. The guard has to live in a function rather than inline in the f-string, because Python would otherwise evaluate the argument on every decode step. The sync perturbs the CPU timeline, not the GPU kernel duration measured. Consumer support is in FaaSApps/rocm-scripts#878, which accepts both the old and new spellings, so neither side has to land first. Co-authored-by: Claude Signed-off-by: Robert Esclapez Garcia <robert.garcia@amd.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two producer-side gaps that stop a W4A16 trace from becoming a bandwidth number.
1. Three spellings of one field. #1067 made the labels self-describing, but the group size is written three ways: bare
g32on the dense scopes,g=32on the MoE ones,gs=/sk=on the AWQ GEMVs. Settle onkey=valueeverywhere.hybrid_w4a16.pycoverswvsplitk_int4,hybrid_dequant_w4a16andhybrid_triton_w4a16in one line — they share the_gzsuffix.2. The MoE scope carried no expert count. Weight slabs dominate that kernel's traffic and it loads one per block, but the count cannot be recovered downstream: a trace records
expert_ids' length, not its contents, and several blocks can share an expert. So the block count is only an upper bound, andE/top_ka looser one.fused_moe_wvsplitk_int4now emits the measured value:On the consumer side that is the difference between 5.23 MiB and 33.08 MiB of modelled traffic for a 32-block launch touching 5 distinct experts — 6.3x, in a byte model whose whole job is to be exact.
Cost when not profiling: none
The count needs a device->host sync, so
_fused_moe_wvsplitk_scope()follows_moe_gemm_w4a16_scope()exactly: it returnscontextlib.nullcontext()before touching the tensor unlessVLLM_CUSTOM_SCOPES_FOR_PROFILING/VLLM_NVTX_SCOPES_FOR_PROFILINGis set.The guard has to live in a helper rather than inline in the f-string — Python evaluates call arguments eagerly, so an inline
bincountwould run on every decode step regardless. The sync perturbs the CPU timeline, not the GPU kernel duration the roofline measures.Verified rather than argued: passing a sentinel whose every accessor raises, the scope returns a
nullcontextuntouched with profiling off. With it on,expert_ids = [3, 3, 7, 7, 7, 11, -1, -1]yieldsexperts=3— distinct ids, duplicates collapsed,-1padding excluded. The block-count bound would have said 8.Not a duplicate
hybrid_w4a16.pytoo, but the load path (process_weights_after_loading), not the profiler labels. Different concern, and complementary: its own description leans on the corrected byte accounting this labelling enables ("every wvsplitk_int4 shape sits at 187-204 GiB/s next to 216 GiB/s for the fp16 lm_head"). Expect a small contextual conflict in that file; happy to rebase behind it.record_functionlabels.Test plan
pytest tests/kernels/quantization/test_hybrid_w4a16_perf.py-> 137 passed, 3 failed.The 3 failures are pre-existing flaky perf assertions, not regressions. Same three tests, 5 repetitions each:
gfx11(base, without this change)5 failures in 15 test-runs on base vs 4 in 15 here — indistinguishable. They also fail in the improvement direction (e.g.
batch_size=1: 0.76 TFLOP/s (expected 0.68 +[-8,8]%) FAIL (improvement, +11.0%)), which a label change cannot cause. This is the golden-reproducibility issue already tracked on #1065 and noted in #1073.That run is also the direct check that the in-repo label consumer still works:
test_hybrid_w4a16_perf.py:627reads the scope name back out of the profiler to report which path ran, and its output tags the rows[wvsplitk_int4]/[hybrid_triton_w4a16]— a broken parse would read[unknown].End-to-end capture on
gemma-4-31B-it-AWQ-4bit(decode,--profile --profile-record-shapes --profile-export-trace), with this branch installed:wvsplitk_int4 1x43008x5376 g=32 asymAlso run:
pre-commit— full hook set passed on commit (ruff, format, sign-off, config validation)Still not run:
VLLM_CUSTOM_SCOPES_FOR_PROFILING=1end to end. No MoE AWQ checkpoint is cached on this machine (only GGUF ones and the dense gemma), so this would need a multi-GB download. The scope function itself is verified directly as described above, and the dense capture proves the label -> trace -> roofline path; what remains unproven is only that the MoE scope's window attaches to the right op in a real trace.(An earlier revision of this PR said the test suite could not run at all. That was wrong: it needed
amdsmionPYTHONPATH— without it vLLM resolves toUnspecifiedPlatformand every gfx1151-gated test skips.)AI assistance
This change was written with AI assistance (Claude). Per
AGENTS.md: the analysis motivating it — the byte accounting, the annotation/consumer contract, the expert-count bound — was developed and verified interactively against real gfx1151 traces, and I have reviewed every changed line. Kept as a draft pending the MoE end-to-end capture above.