Batch-invariance generation with vLLM fused-MoE backend at improved performance - #6521
Conversation
|
This PR has been automatically converted to draft because all PRs must start as drafts. When you are ready for review, click Ready for Review to begin the review process. This will:
See the contribution guide for more details. |
|
/claude strict-review |
| return adjusted_batch_dim | ||
|
|
||
|
|
||
| def _batch_invariant_token_floor(token_count: int) -> int: |
There was a problem hiding this comment.
I think this is un-needed for the old BiK paths if we are actually overriding the norms but I dont think this will make much of a difference for perf regardless to I am ok activating every time
There was a problem hiding this comment.
Yeah you are right. The floor is only load-bearing for the te_native backend (native rmsnorm's M%32 bit-class); with the deepgemm/triton backends the substituted norm makes it redundant. I kept it unconditional so bucket geometry stays uniform across backends (and the collapsed 1/2/4-token buckets slightly reduce graph count/capture time), but happy to gate it on te_native if you'd prefer.
| _TE_NATIVE_WORKSPACE_BYTES = 1024 | ||
|
|
||
|
|
||
| def _enable_te_native_workspace_starvation(workspace_bytes: int = _TE_NATIVE_WORKSPACE_BYTES): |
There was a problem hiding this comment.
Does this work on all gpu types(hopper, Blackwell, Rubin)?
There was a problem hiding this comment.
I tested right now on Blackwell, but vLLM similarly guards this only for Hopper and Blackwell: https://github.com/vllm-project/vllm/blob/main/vllm/model_executor/layers/batch_invariant.py#L915-L927
There was a problem hiding this comment.
I dont think we need to care about ampere but I guess we could guard for when we use Rubin until we test
|
For the most part looks good to me. Only question is around do we know this path will work on hopper Blackwell and Rubin? |
| def add_if_valid(token_count: int, prefill_req_count: int, decode_req_count: int) -> None: | ||
| """Helper to create and append batch dimension to list only if it's valid.""" | ||
| if _batch_invariant_mode_enabled(): | ||
| # Batch-invariant mode: floor EVERY bucket's token count to a | ||
| # 64-multiple (see _batch_invariant_token_floor); the flooring | ||
| # can collide previously-distinct buckets, so skip duplicates. | ||
| token_count = _batch_invariant_token_floor(token_count) | ||
| batch_dim = InferenceBatchDimensions(token_count, prefill_req_count, decode_req_count) | ||
| if batch_dim.is_valid(max_requests, max_sequence_length, num_speculative_tokens): | ||
| cuda_graph_batch_dimensions_list.append(batch_dim) | ||
| if batch_dim not in cuda_graph_batch_dimensions_list: | ||
| cuda_graph_batch_dimensions_list.append(batch_dim) |
There was a problem hiding this comment.
[CRITICAL Correctness] Rounding token_count up here silently deletes decode buckets instead of aligning them, whenever the request count is not a multiple of 64.
_batch_invariant_token_floor rounds up to the next 64-multiple, but InferenceBatchDimensions.is_valid rejects any dims where
token_count > prefill_req_count * max_sequence_length
+ decode_req_count * (num_speculative_tokens + 1)For the decode-only call sites (prefill_req_count=0, lines ~562-568 and ~629-635) the right-hand side is exactly decode_req_count * (spec + 1), which is the pre-rounding token_count. So as soon as the floor bumps the value, the bucket fails validation and is dropped by the if batch_dim.is_valid(...) guard — no warning, no fallback bucket.
Concretely, with max_requests=100, num_speculative_tokens=0, tp_size=1: decode_req_count = 100, token_count = 100 → floored to 128 → 128 > 100 * 1 → the largest decode graph is never captured. Every decode step at high concurrency then falls back to eager, which both costs the throughput the PR is claiming and — more importantly — puts those steps on a different code path from the graphed ones, which is the opposite of what batch-invariant mode is for. The same happens for any auto-computed bucket size that isn't a 64-multiple.
TestCudaGraphBucket64Floor can't catch this: it only asserts token_count % 64 == 0 over surviving buckets and uses max_requests=512, itself a 64-multiple, so nothing is ever dropped.
Fix: raise the request budget along with the token count so the invariant still holds, and clamp to max_requests so we degrade to a smaller aligned bucket rather than to nothing:
def add_if_valid(token_count: int, prefill_req_count: int, decode_req_count: int) -> None:
"""Helper to create and append batch dimension to list only if it's valid."""
if _batch_invariant_mode_enabled():
aligned = _batch_invariant_token_align(token_count)
if prefill_req_count == 0 and num_speculative_tokens == 0:
# Decode-only dims are validated against decode_req_count, so the
# request budget has to grow with the aligned token count or the
# bucket is silently rejected below.
decode_req_count = min(aligned, max_requests)
aligned = min(aligned, decode_req_count)
token_count = aligned
...and extend the test with a non-64-multiple max_requests (e.g. 100) asserting the largest decode bucket still exists.
There was a problem hiding this comment.
Fixed in 8d2f97d. Went with validate-then-align rather than raising the request budget: the alignment is padding (same semantics as the eager TOKEN_ROUNDER), so validity is checked on the unpadded dims and the request count stays truthful; note the proposed min(aligned, decode_req_count) clamp would un-align the tokens again. Added the max_requests=100 regression test asserting the largest decode bucket survives padded.
|
Strict review summary Findings: CRITICAL: 2 · IMPORTANT: 3 · SUGGESTION: 5 The core idea of this PR is sound, and the parts that matter most for bit-equality — the Inductor-verbatim fp32 SwiGLU sequence with a single final bf16 round, the gate|up half-split convention matching Most impactful findings 1. Decode CUDA-graph buckets are silently dropped, not aligned ( 2. 3. The Also flagged: Risk assessment Medium-high, concentrated in the CUDA-graph sizing change. Everything gated behind |
Factor the batch-invariant alignment multiple into a module-level TOKEN_ROUNDER in batch_dimensions_utils, used by all three batch-invariant sites (align function, token ladder, smallest bucket). DynamicInferenceContext.TOKEN_ROUNDER now references the same constant (that import direction already exists), so the eager rounder and the batch-invariant alignment share one definition and the earlier lazy import is removed. Behavior-identical. GPU-validated: 24/24 (job 2403997). Signed-off-by: Utkarsh Utkarsh <uutkarsh@nvidia.com>
Signed-off-by: Utkarsh Utkarsh <uutkarsh@nvidia.com>
Head branch was pushed to by a user without write access
82befef to
8a25689
Compare
|
/ok to test 8a25689 |
The config-drift guard in test_hybrid_moe_model.py snapshots every TransformerConfig field; register the new batch_invariant_backend field with its default value (te_native), following the test's ADDED ARGS guidance. The field only takes effect when batch_invariant_mode is enabled, so downstream model configs are unaffected. Signed-off-by: Utkarsh Utkarsh <uutkarsh@nvidia.com>
|
/ok to test a5aa088 |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/32581507186 |
…g GTP to dev An AST scan for undefined names over the 137 files this branch touches found six sites where a symbol is referenced but never bound. origin/main is clean at all six, so every one is an artifact of a conflict resolution in this port, not an upstream defect. They fall into two kinds. Dropped: a picked commit's call site arrived while its definition did not. optimizer.py NVIDIA#4967 uses chain.from_iterable but lost the `from itertools import chain` that main carries. resharding/execution.py NVIDIA#6133 kept the refresh_module_caches() call and lost the function. resharding/planner.py NVIDIA#6133 lost TensorReshardSpec from the .utils import, plus the _NativeParameterPart dataclass and _find_source_metadata(); resharding/utils.py lost TensorReshardSpec itself and ReshardPlan's two tensor_reshard_* fields. tests/.../test_planner.py lost three names from its import block. emerging_optimizers.py NVIDIA#6664 kept `**ns_kwargs` at the newton_schulz_tp call and lost the assignment above it. use_syrk is plumbed through this file, so restoring the assignment (rather than deleting the kwarg) is what keeps --muon-use-syrk from silently doing nothing. Leaked: a fragment of a PR this branch never picked rode in on a neighbouring hunk. batch_invariant comes from NVIDIA#6521/NVIDIA#4871, neither of which is in this stack, and megatron/core/inference/moe/batch_invariant.py does not exist here. fused_moe.py passed return_batch_invariant_inverse_map=<undefined>. permute.py declared that parameter while its kernel has no HAS_INVERSE path, i.e. it advertised a capability the body cannot deliver. Both removed, restoring dev's behaviour exactly. Not touched: attention.py reports the same class of error for MultimodalRotaryEmbedding/YarnRotaryEmbedding, but origin/dev reports it too, so it is upstream's and outside this port's scope. Compile alone cannot catch any of this -- every one of these files parses. Signed-off-by: xuwenc <xuwenc@nvidia.com>
…erformance (NVIDIA#6521) Signed-off-by: Utkarsh Utkarsh <uutkarsh@nvidia.com> Co-authored-by: Utkarsh Utkarsh <uutkarsh@nvidia.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Batch-invariant mode: vLLM fused-MoE backend,
te_nativeGEMM backend, and gated-model supportSummary
batch_invariant_modecurrently forces the inference-optimized MoE onto the torch/DeepGEMMbackend, which costs ~6× generation throughput on the same engine, and it cannot run
gated-activation MoE models (e.g. Qwen3-MoE — the models #5700 enabled) at all. This PR makes
the vLLM Triton fused-MoE backend batch-invariant, adds a zero-overhead
te_nativeGEMMbackend, and fixes gated-SwiGLU support under the flag — bringing batch-invariant generation
to parity with the engine's non-invariant throughput.
Measured on Qwen3-30B-A3B, EP8/TP1, 8×B200, full CUDA graphs, BS256/OSL1024:
te_native)What's included
2-token decode buckets whose graphed norms execute in a different M%32 reduction bit-class
than eager steps (TOKEN_ROUNDER=64), breaking cross-batch bit-equality. All sizing
distributions floored under BI mode; request counts untouched.
squared-ReLU; gated models crashed with a grouped-GEMM K mismatch. Adds
swiglu_with_probs(graph-safe, training-parity rounding).
(
BLOCK_SIZE_K; the kernel is fp32-accumulate with no split-K, so M/N tiling and pipelinedepth cannot affect bits and stay hint-adaptive), applies routing probabilities at the
activation with the training kernel's exact rounding (device-bounded, CUDA-graph-safe
kernel), and reduces top-k with unit weights in fp64.
te_nativeGEMM backend — keeps the native cuBLASLt kernels for every dense GEMM(aten and TE) and obtains batch invariance via cuBLASLt workspace starvation: split-K
variants require workspace, so a ~1KB workspace pins every M to the same serial-K reduction
recipe at native speed. Includes a workaround for TE (≤2.15 verified) hardcoding a 32MiB
workspace and ignoring
CUBLASLT_WORKSPACE_SIZE. Underte_native, native TE RMSNorm isalso kept (the 64-multiple alignment discipline holds its M%32 bit-class constant).
TransformerConfig.batch_invariant_backend(
"deepgemm" | "triton" | "te_native") plumbed through--batch-invariant-backend.tests/unit_tests/inference/test_vllm_fused_moe_batch_invariant.py: bucket-floorproperties across all sizing distributions, kernel repeat-determinism / row-locality /
device-bound soundness under NaN-poisoned tails,
_moe_sumoption coverage, an end-to-endbitwise invariance test (same tokens across different co-batch sizes and launch-config
classes →
torch.equal), and te_native enable/disable round-trip. 11/11 passing;the pre-existing BI/inference suites show zero regressions vs stock main
(identical pass/skip/fail sets in the same environment).
Usage
Issue tracking
For PRs from open-source community contributors:
Linked issue:
Contribution process
Pre-checks
Code review
Feel free to message or comment @NVIDIA/mcore-oncall to help accelerate your merge into main. The less complex your PR is, the faster it will be approved and merged!
All PRs start as draft. If you open a non-draft PR, it will be automatically converted to draft.
Step 1: Mark PR as "Ready for Review"
.github/CODEOWNERS.Final Review might get declined if these requirements are not fulfilled.
Step 2: Final Review
For PRs that change
megatron/core, once all expert reviewers have approved, theFinal Reviewlabel is applied automatically and final reviewers are assigned.For PRs outside
megatron/core, this step is skipped.Step 3: Approved
Once all required reviewers have approved, the
Approvedlabel is applied automatically.Merge
Any member of mcore-engineers will be able to merge your PR.