[GG] Prewarm CuTe PCIe one-shot all-reduce before graph capture - #248
Conversation
|
Warning Review limit reached
Next review available in: 52 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughCUDA graph warmup now prepares PCIe graph all-reduce for eligible plain inputs. Tests verify preparation on the capture stream and confirm that DMA-only inputs skip graph preparation. ChangesCustom all-reduce warmup
Estimated code review effort: 3 (Moderate) | ~15–30 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/distributed/test_b12x_custom_all_reduce_warmup.py (1)
77-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winVerify that DMA-only warmup does not communicate.
Assert that
custom_allreduce._pcie_dma.all_reducewas not called. Also assert thatoutput.shapematchesinp.shape. The current assertions pass if a future change launches DMA communication or returns a tensor with the wrong shape.Proposed test additions
assert output is not None + assert output.shape == inp.shape runtime.prepare_graph_all_reduce.assert_not_called() + runtime.all_reduce.assert_not_called() + custom_allreduce._pcie_dma.all_reduce.assert_not_called()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/distributed/test_b12x_custom_all_reduce_warmup.py` around lines 77 - 80, Strengthen the test around custom_allreduce.custom_all_reduce by asserting that custom_allreduce._pcie_dma.all_reduce was not called and that output.shape equals inp.shape, while retaining the existing output and runtime.prepare_graph_all_reduce assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/distributed/test_b12x_custom_all_reduce_warmup.py`:
- Around line 12-14: Add Google-style docstrings to _make_custom_allreduce, the
capture-warmup mock helper, and the plain-input and DMA-only test functions in
tests/distributed/test_b12x_custom_all_reduce_warmup.py at lines 12-14, 31-34,
50-52, and 68-70; document parameters with Args:, return values with Returns:
where applicable, and exceptions with Raises: where applicable.
---
Nitpick comments:
In `@tests/distributed/test_b12x_custom_all_reduce_warmup.py`:
- Around line 77-80: Strengthen the test around
custom_allreduce.custom_all_reduce by asserting that
custom_allreduce._pcie_dma.all_reduce was not called and that output.shape
equals inp.shape, while retaining the existing output and
runtime.prepare_graph_all_reduce assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fa8f3094-b9f6-4b78-8c53-e08e338980a6
📒 Files selected for processing (2)
tests/distributed/test_b12x_custom_all_reduce_warmup.pyvllm/distributed/device_communicators/custom_all_reduce.py
28b5ec0
into
local-inference-lab:dev/gilded-gnosis
Summary
Prewarm the B12X CuTe one-shot all-reduce specialization during vLLM's CUDA graph warmup, before the nested Inductor capture begins.
B12X commit
680d819migrated the PCIe communication kernels to CuTe DSL and correctly rejects a cold one-shot launch during CUDA graph capture. vLLM's existing graph-warmup branch intentionally skipped the collective and returnedempty_like; that preserved communication semantics, but it also meant the new CuTe specialization was never prepared. A cold GLM TP4/DCP4 boot then failed in the first PIECEWISE graph capture with:Change
When vLLM is in the non-capturing warmup branch:
Validation
2 passedall_reduce;42/42exact responses passed.Runtime gate used current GG plus current B12X master and the GLM TP4/DCP4 one-shard query-split profile.
Summary by CodeRabbit