[https://nvbugs/6572835][fix] Use HashStore for single-rank MegaMoE tests - #17497
Conversation
8ca5414 to
06ab6ee
Compare
|
/bot run --disable-fail-fast |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan includes up to 12 reviews per rolling hour; 10 remain after this review. WalkthroughThe changes update single-process MegaMoE tests and benchmarks to use local ChangesMegaMoE test setup
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change limits single-rank MegaMoE tests to in-process rendezvous while preserving multi-rank behavior, with no actionable merge-blocking risk remaining after normal checks and review. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
PR_Github #65288 [ run ] triggered by Bot. Commit: |
|
PR_Github #65288 [ run ] completed with state
|
brnguyen2
left a comment
There was a problem hiding this comment.
Approving — the comments below are optional touch-ups, not blockers.
Right fix for the right reason — a single-rank group needs a ProcessGroup, not a rendezvous, so removing the socket entirely is stronger than probing a free port. Verified that dropping the RANK/WORLD_SIZE/LOCAL_RANK env writes in test_moe_backend.py is safe: both MegaMoE bootstraps (mega_moe_cute_dsl.py:782, mega_moe_deepgemm.py:392) early-return when the MPI world is 1, so nothing downstream reads them in these tests.
Two follow-ups below, neither blocking. Since the waiver removal is the point of the PR, please confirm the unwaived -k "MEGAMOE_CUTEDSL" case actually ran and passed in this PR's B200 stage before merging rather than only that the pipeline was green.
|
/bot run --disable-fail-fast |
|
PR_Github #65534 [ run ] triggered by Bot. Commit: |
|
PR_Github #65534 [ run ] completed with state
|
118a2b3 to
168f20c
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/bot run --disable-fail-fast |
|
PR_Github #65851 [ run ] triggered by Bot. Commit: |
|
PR_Github #65851 [ run ] completed with state
|
168f20c to
4ae87a5
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #66618 [ run ] triggered by Bot. Commit: |
|
PR_Github #66618 [ run ] completed with state
|
…ests Signed-off-by: Barry Kang <43644113+Barry-Delaney@users.noreply.github.com>
Signed-off-by: Barry Kang <43644113+Barry-Delaney@users.noreply.github.com>
4ae87a5 to
41512e6
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #66681 [ run ] triggered by Bot. Commit: |
|
PR_Github #66681 [ run ] completed with state |
Dev Engineer Review
torch.distributed.HashStore.env://rendezvous failures whenMASTER_PORTis occupied.MEGAMOE_CUTEDSLwaiver removal is valid and enables existing CI coverage.QA Engineer Review
test_moe_backend.pytest_moe_module.pytests/microbenchmarks/bench_moe/utils.pytest_configurable_moe_single_gpu -k "MEGAMOE_CUTEDSL"inl0_b200.yml.test_moe_backend -k "MEGAMOE_CUTEDSL"inl0_b200.yml.MEGAMOE_CUTEDSLwaiver fromtests/integration/test_lists/waives.txt.MEGAMOE_CUTEDSLremains covered and waived separately.ci: full pre-merge approvedlabel.Description
The single-rank MegaMoE unit-test helpers initialize an NCCL process group through the default
env://rendezvous. In CI this inheritsMASTER_PORT, so an unrelated listener occupying that port causes c10d TCPStore to fail withEADDRINUSEbefore the MegaMoE test executes.Use an in-process
torch.distributed.HashStorewhenworld_size == 1. Single-rank tests require a ProcessGroup but do not need cross-process rendezvous, so no network socket is necessary. Multi-rank tests retain the existing shared TCP rendezvous path.Apply the same behavior to both MegaMoE single-rank test helpers and remove the NVBug 6572835 waiver so the affected test runs in CI again.
Test Coverage
MASTER_PORTdeliberately occupied: the original helper reproducedEADDRINUSE, while both patched helpers initialized NCCL and completedall_reducepre-commit run --files tests/unittest/_torch/modules/moe/test_moe_module.py tests/unittest/_torch/modules/moe/test_moe_backend.py tests/integration/test_lists/waives.txtgit diff --checkPR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit summary, please make sure it makes sense.
PR follows TRT-LLM coding guidelines to the best of your knowledge.
Test cases are provided for new code paths.
No public API, dependency, ownership, or architecture changes are introduced.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.