Skip to content

fix(ascend): use CommInitAll for single-node TP - #566

Merged
voltjia merged 1 commit into
refactor/adopt-modern-infini-stackfrom
fix/ascend-single-node-tp
Sep 10, 2026
Merged

fix(ascend): use CommInitAll for single-node TP#566
voltjia merged 1 commit into
refactor/adopt-modern-infini-stackfrom
fix/ascend-single-node-tp

Conversation

@baominghelly

Copy link
Copy Markdown
Contributor

Summary

  • Use infinicclCommInitAll for Ascend single-node tensor-parallel communicator initialization.
  • Keep the existing GetUniqueId / CommInitRank path for platforms that support rank-based initialization.
  • Add a static contract test requiring Ascend single-node TP to use CommInitAll rather than CommInitRank.

Motivation

The current Ascend HCCL backend in InfiniCCL supports CommInitAll, AllReduce, and CommDestroy, but does not support GetUniqueId or CommInitRank. As a result, Ascend TP initialization fails with:

RuntimeError: InfiniCCL operation `infinicclGetUniqueId` failed with result 8

This change aligns InfiniLM with the current Ascend InfiniCCL contract. It depends on the Ascend HCCL backend changes tracked in InfiniCCL PR #73.

Type of Change

  • fix — bug fix

Test Results of Involved Models on Supported Platforms

Tested on Ascend 910C with the modern InfiniRT / InfiniOps / InfiniCCL stack and InfiniLM refactor/adopt-modern-infini-stack at 3faa1db.

Verified communicator initialization:

  • TP2: established successfully.
  • TP4: established successfully.
  • TP8: established successfully.

Representative inference results after the fix:

  • FM9G_70B, flash/paged attention, TP8: command completed and generated coherent Chinese output.
  • Qwen3-32B, flash/paged attention, TP4: generated a valid thinking block and final answer.
  • Llama-3.3-70B-Instruct, flash/paged attention, TP8: generated coherent Chinese output.
  • Baichuan2-7B-Chat, flash/paged attention, TP2: generated a coherent factual response.

Static test:

python test/static/test_infinicore_runtime_contracts.py \
  InfiniCoreRuntimeContractsTest.test_ascend_single_node_tp_uses_comm_init_all

Ran 1 test in ... OK

Benchmark / Performance Impact

N/A. This is a correctness fix required before Ascend TP inference can initialize.

Notes for Reviewers

  • This PR intentionally only changes communicator initialization and its contract test.
  • Dense-prefill correctness changes and Ascend graph capture diagnostics are kept as separate follow-ups.
  • Cross-node pipeline-parallel initialization still uses the rank-based path through initializeCommunicators; this PR only changes the intra-node TP path.

CI / ChatOps

CI needs to be triggered manually from the Actions tab, or by a maintainer with /retest or /test.

Checklist

Title, Branch, and Commits

  • PR title follows Conventional Commits.
  • Branch name follows <type>/xxx-yyyy-zzzz.
  • Commit message follows Conventional Commits.
  • Small PR is a single squashable commit.
  • No stray merge commits from main.
  • No fixup! / squash! / wip commits remain.
  • Existing PR/branch/commit that followed the legacy issue format.

Scope and Design

  • Changes are minimal and scoped to Ascend TP initialization.
  • No dead code, debug prints, or unrelated formatting churn.
  • No public API changes.

General Code Hygiene

  • Comments are limited to the non-obvious platform contract.
  • Files end with a newline.
  • No trailing whitespace or BOMs.
  • Comments and identifiers use English and backticks where appropriate.

C++ Specific

  • Code follows the existing style.
  • No new raw new / delete.
  • Changed files are formatted according to the existing source style.
  • No changes to csrc/models/llama_legacy/.

Python Specific

  • Test code follows the existing static contract-test style.

Testing

  • Affected platform tested on Ascend 910C.
  • Passed representative single-request and offline inference checks for TP2/TP4/TP8.
  • Sanity and service suites not run because this draft is narrowly scoped to communicator initialization.

Build, CI, and Tooling

  • Incremental release build and _infinilm install passed on Ascend.
  • CI still needs to be triggered manually.

Documentation

  • No developer workflow or public behavior documentation change is needed.

Security and Safety

  • No secrets, internal URLs, customer data, or personal hardware identifiers are committed.
  • No third-party code was introduced.
  • No unsafe pointer arithmetic or missing bounds checks were introduced.

@baominghelly
baominghelly force-pushed the fix/ascend-single-node-tp branch from f5f8464 to 561c04c Compare September 10, 2026 03:24
@baominghelly
baominghelly marked this pull request as ready for review September 10, 2026 06:15
@voltjia
voltjia merged commit b6fbf94 into refactor/adopt-modern-infini-stack Sep 10, 2026
6 checks passed
@voltjia
voltjia deleted the fix/ascend-single-node-tp branch September 10, 2026 07:05
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.

2 participants