Skip to content

add hccl extension#571

Open
PKUZHOU wants to merge 3 commits intohw-native-sys:mainfrom
PKUZHOU:pr1-dist-base
Open

add hccl extension#571
PKUZHOU wants to merge 3 commits intohw-native-sys:mainfrom
PKUZHOU:pr1-dist-base

Conversation

@PKUZHOU
Copy link
Copy Markdown

@PKUZHOU PKUZHOU commented Apr 15, 2026

No description provided.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements a distributed communication framework for the A2A3 platform, supporting HCCL hardware and shared-memory simulation backends. It introduces a per-chip bootstrap mechanism for communicator initialization, device memory allocation, and data staging, supported by the new DistChipBootstrapChannel. The Worker class and CodeRunner utility were refactored to handle level 3 distributed execution and process lifecycle management, with corresponding extensions to the Python bindings and C API. Feedback identifies a breaking change in the task.orch function signature and suggests optimizing performance by moving a module import out of a loop in the bootstrap method. Comprehensive unit tests for the bootstrap channel and worker API are also included.

Comment thread python/simpler/worker.py Outdated
self._orch._scope_begin()
try:
task.orch(self._orch, task.args)
task.orch(self, task.args)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The signature of the orchestrator function task.orch has been changed from orch(orchestrator, args) to orch(worker, args). This is a significant breaking change that should be documented in the pull request description. While the new API of calling methods on the worker instance is an improvement, the lack of documentation for this change can cause issues for users of this class.

if len(blob) != size:
raise ValueError("input blob size must match buffer size")
if size > 0:
import ctypes as _ct
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The import ctypes as _ct statement is inside a loop. While this works, it's inefficient to re-import the module on every iteration. For better performance and code style, please move this import to the top of the bootstrap method (e.g., on line 304).

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