Skip to content

[None][feat] Update default tactic allreduce strategy for SM103 - #18510

Closed
Wanli-Jiang wants to merge 1 commit into
NVIDIA:mainfrom
Wanli-Jiang:user/williamj/qwen38-flash-next-f0-auto-allreduce
Closed

Wanli-Jiang wants to merge 1 commit into
NVIDIA:mainfrom
Wanli-Jiang:user/williamj/qwen38-flash-next-f0-auto-allreduce

Conversation

@Wanli-Jiang

@Wanli-Jiang Wanli-Jiang commented Sep 1, 2026 •

Copy link
Copy Markdown
Collaborator

Dev Engineer Review

  • Updated the default all-reduce tactic policy for SM103.
  • SM103 now selects NCCL instead of NCCL symmetric when no cached tactic exists.
  • Added capability checks for NCCL symmetric auto tactics and symmetric-memory windows.
  • Preserved explicit NCCL symmetric selection and fallback behavior when device discovery is unavailable.
  • Added compile-time assertions for the SM103 and non-SM103 policies.
  • No public API or exported declaration changes were identified.
  • The changes are consistent across the C++ and Python implementations.

QA Engineer Review

  • Added test functions:
    • test_sm103_excludes_nccl_symmetric_from_auto
    • test_other_capability_preserves_nccl_symmetric_auto
    • test_unavailable_capability_preserves_existing_policy
    • test_sm103_auto_tactics_and_cache_miss_use_safe_collectives
    • test_non_sm103_auto_preserves_symmetric_tactic_and_fallback
    • test_sm103_auto_does_not_request_nccl_window_output
    • test_non_sm103_auto_preserves_nccl_window_output
  • These tests cover SM103 behavior, non-SM103 behavior, unavailable device discovery, tactic filtering, cache-miss fallback, and symmetric-memory-window selection.
  • No tests/integration/test_lists/ coverage entry was provided for these unit tests.
  • Verdict: needs follow-up.

Description

Test Coverage

PR Checklist

Please review the following before submitting your PR:

  • PR description clearly explains what and why. If using CodeRabbit's 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 (see test instructions)

  • If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.

  • Any new dependencies have been scanned for license and vulnerabilities

  • CODEOWNERS updated if ownership changes

  • Documentation updated as needed

  • Update tava architecture diagram if there is a significant design change in PR.

  • The reviewers assigned automatically/manually are appropriate for the PR.

  • 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.

Signed-off-by: Wanli Jiang <35160485+Wanli-Jiang@users.noreply.github.com>
@Wanli-Jiang

Copy link
Copy Markdown
Collaborator Author

verified not necessary, close the PR.

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8de9954d-3a43-4317-83a2-5e9f22722c3a

📥 Commits

Reviewing files that changed from the base of the PR and between f04859d and 82bc0ce.

📒 Files selected for processing (4)
  • cpp/tensorrt_llm/thop/allreduceOp.cpp
  • tensorrt_llm/_torch/custom_ops/torch_custom_ops.py
  • tensorrt_llm/_torch/distributed/ops.py
  • tests/unittest/_torch/distributed/test_allreduce_auto_policy.py

Walkthrough

AllReduce AUTO tactic selection now checks NCCL symmetric support. SM103 uses NCCL instead of NCCL symmetric, including cache-miss fallbacks and memory-window selection. Other architectures preserve the existing symmetric behavior.

Changes

AllReduce policy

Layer / File(s) Summary
Capability and window policy
tensorrt_llm/_torch/distributed/ops.py, tests/unittest/_torch/distributed/test_allreduce_auto_policy.py
AUTO NCCL symmetric support is disabled on SM103. Explicit NCCL strategies remain eligible. Tests cover supported, unsupported, and unavailable device capability cases.
Python tactic selection and fallback
tensorrt_llm/_torch/custom_ops/torch_custom_ops.py, tests/unittest/_torch/distributed/test_allreduce_auto_policy.py
The runner filters unsupported NCCL symmetric tactics and selects NCCL or NCCL symmetric for cache misses according to platform support.
C++ cache-miss fallback
cpp/tensorrt_llm/thop/allreduceOp.cpp
The uncached autotuned path selects NCCL on SM103 and NCCL symmetric on other architectures.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: bowenfu

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant