add dsl algorithms support to bench collective by Empyreus · Pull Request #887 · microsoft/mscclpp

RJ Souza (Empyreus) · 2026-08-25T21:10:40Z

add dsl algorithms support to bench collective
replace multinode pipeline tests with bench_collective tests

azure-pipelines · 2026-08-25T21:10:49Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

Copilot

Pull request overview

Adds multi-node DSL collective autotuning and CI coverage for AllReduce, AllGather, and ReduceScatter.

Changes:

Compiles and tunes runtime DSL variants with configurable launch geometry.
Adds ReduceScatter benchmarking and correctness validation.
Reworks multi-node CI to exercise DSL collectives.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`test/deploy/run_tests.sh`	Adds manual multi-node DSL test helpers.
`python/mscclpp/default_algos/reducescatter_multi_nodes.py`	Adds a plan-generation CLI.
`python/mscclpp_benchmark/tuner.py`	Supports candidate-specific tuning dimensions.
`python/mscclpp_benchmark/correctness.py`	Validates ReduceScatter output.
`python/mscclpp_benchmark/comm.py`	Compiles and executes DSL variants.
`python/mscclpp_benchmark/bench_collective.py`	Adds DSL candidates and ReduceScatter benchmarking.
`.azure-pipelines/multi-nodes-test.yml`	Adds three multi-node DSL benchmark jobs.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Suppressed comments (2)

python/mscclpp_benchmark/comm.py:203

Every compiled DSL plan is hard-coded as in-place, but --enable-dsl can be combined with --buffer-mode out-of-place, and _dsl_candidate_specs does not filter by algorithm.buffer_mode. For out-of-place AllReduce/AllGather, the tuner therefore executes plans whose buffer layout assumes aliasing, causing incorrect output before falling into its default fallback. Either compile the requested mode where the builder supports it, or reject/filter DSL candidates whose mode does not match the case.

                    collective=collective_op,
                    nranks_per_node=nranks_per_node,
                    world_size=world_size,
                    in_place=True,

python/mscclpp_benchmark/bench_collective.py:405

The accepted buffer_mode argument is ignored for ReduceScatter: requesting out-of-place still returns an aliased input/output case. This silently benchmarks a different layout than the CLI requested. Since the registered ReduceScatter DSL algorithm is in-place-only, reject this combination explicitly.

    if collective == _REDUCESCATTER:
        # The DSL reducescatter is compiled in-place, so the per-rank output chunk always aliases the
        # matching slice of the full input buffer (mirrors python/test/executor_test.py build_bufs).
        input_buffer = _mscclpp().GpuBuffer(nelems * comm_group.nranks, dtype=dtype_spec.cupy_dtype)

azure-pipelines · 2026-08-28T18:07:46Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

…nto rjsouza/multinode-ci

Copilot

🟡 Changes recommended

BF16 ReduceScatter validation is incorrect, multi-node safety is bypassable, and the new CI checks are non-gating.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

python/mscclpp_benchmark/bench_collective.py:506

ReduceScatter silently ignores an out-of-place request and returns a case labeled in-place. Meanwhile Comm skips compiling ReduceScatter DSL plans for that requested mode, so autotuning reports no candidates and exits successfully without benchmarking anything. Reject this unsupported mode explicitly (or implement matching out-of-place buffers).

    if collective == _REDUCESCATTER:
        # The DSL reducescatter is compiled in-place, so the per-rank output chunk always aliases the
        # matching slice of the full input buffer (mirrors python/test/executor_test.py build_bufs).
        input_buffer = _mscclpp().GpuBuffer(nelems * comm_group.nranks, dtype=dtype_spec.cupy_dtype)

.azure-pipelines/multi-nodes-test.yml:110

With continueOnError: true, failures from this new AllGather correctness benchmark do not fail the pipeline. Since this step is intended as a multi-node test, it needs to be gating.

      name: RunBenchCollectiveAllgatherTest
      displayName: Run multi-nodes bench_collective DSL allgather
      continueOnError: true

.azure-pipelines/multi-nodes-test.yml:128

With continueOnError: true, failures from this new ReduceScatter correctness benchmark do not fail the pipeline. Since this step is intended as a multi-node test, it needs to be gating.

      name: RunBenchCollectiveReduceScatterTest
      displayName: Run multi-nodes bench_collective DSL reducescatter
      continueOnError: true

Files reviewed: 6/6 changed files
Comments generated: 4
Review effort level: Balanced

+        # The tuner falls back here when every candidate fails, which bypasses the multi-node filter
+        # in _candidate_algorithms. Only the compiled DSL plans work across nodes; the native
+        # algorithms _default_tuned_config prefers are single-node CUDA-IPC and would hang.
+        if self._comm_group.nranks > self._comm_group.nranks_per_node:
+            if default_config.algorithm not in self._dsl_algorithms:


+        reduced = sum(values.astype(cp.float32).reshape(-1)[start : start + chunk] for values in encoded_inputs)
+        return _encode_reduced_output(case, reduced), None


-      displayName: Run multi-nodes mscclpp-test
+      name: RunBenchCollectiveAllreduceTest
+      displayName: Run multi-nodes bench_collective DSL allreduce
      continueOnError: true


+        for tbg in tbg_values:
+            for tpb in tpb_values:
+                spec = AlgoSpec(


Binyang Li (Binyang2014)

Can we move DSL related function to another python file? Make it easy to understand

Binyang Li (Binyang2014) · 2026-09-10T02:56:22Z

+        # DSL algorithms execute through the Executor and fail without one; native algorithms
+        # ignore it, so it is safe to pass unconditionally in run().


Remove this comment?

Binyang Li (Binyang2014) · 2026-09-10T02:58:13Z

+            # reducescatter_multi_nodes lays out its thread block groups from this value.
+            pass_thread_block_group_size = True
+        else:
+            return


Log and throw error here?

Binyang Li (Binyang2014) · 2026-09-10T03:02:01Z

+            builder = allgather_multi_nodes
+            collective_op = AllGather(world_size, 1, in_place)
+            name_prefix = "dsl_allgather"
+            tags = {"default": 1}


Why set tags here

Binyang Li (Binyang2014) · 2026-09-10T03:07:07Z

        dtype_override: Any | None = None,
        accum_dtype: Any | None = None,
        symmetric_memory: bool = False,
    ) -> TunedConfig:


Do we need to change tuned_config to support dsl algo? Now how to persist dsl algo parameters to tuned config

Binyang Li (Binyang2014) · 2026-09-10T03:08:45Z

+                    num_threads_per_block=tpb,
+                    reuse_resources=True,
+                    use_double_scratch_buffer=True,
+                    min_message_size=tbg * (1 << 10),


Why min message size is tbg * (1 << 10)

RJ Souza (Empyreus) and others added 15 commits August 14, 2026 22:59

initial changes

9a532b8

test allreduce

1910eaa

add allgather

268d655

debug issues

9bb2859

Merge branch 'main' into rjsouza/multinode-ci

5806653

rework and add reducescatter test

339fc3a

add dsl to bench collective

2f1e08c

add bench test to pipeline

e124217

Merge branch 'main' into rjsouza/multinode-ci

aa63065

add allreduce

1dd4431

formatting fix

97fa6b9

reducescatter

00e19f7

reducescater

7aca389

Merge branch 'main' into rjsouza/multinode-ci

2411812

setup final multinode-ci

0f9571b

RJ Souza (Empyreus) requested a balanced review from Copilot August 25, 2026 21:11

Copilot started reviewing on behalf of RJ Souza (Empyreus) August 25, 2026 21:11 View session

default reducescatter

4f1fc09

Copilot AI reviewed Aug 25, 2026

View reviewed changes

RJ Souza (Empyreus) added 2 commits August 25, 2026 22:36

revert changes in run_tests.sh

59a07c7

force inplace for reduce_scatter

182b051

RJ Souza (Empyreus) changed the title ~~Rjsouza/multinode ci~~ add dsl algorithms support to bench collective Aug 25, 2026

RJ Souza (Empyreus) added 2 commits August 26, 2026 23:01

fix partial fairure

b0063c6

fix message size

8b74cf7

RJ Souza (Empyreus) requested a balanced review from Copilot August 27, 2026 16:25

Copilot started reviewing on behalf of RJ Souza (Empyreus) August 27, 2026 16:26 View session

Copilot AI reviewed Aug 27, 2026

View reviewed changes

Comment thread python/mscclpp/language/collectives.py Outdated

Comment thread python/mscclpp_benchmark/bench_collective.py

Comment thread python/mscclpp_benchmark/bench_collective.py Outdated

RJ Souza (Empyreus) added 2 commits August 28, 2026 18:05

colpit pr check

f236f40

Merge branch 'main' into rjsouza/multinode-ci

74422f7

RJ Souza (Empyreus) marked this pull request as ready for review August 28, 2026 18:07

RJ Souza (Empyreus) added 6 commits August 28, 2026 11:12

formatting

01b13b0

set total size

c5f7f01

Merge branch 'rjsouza/multinode-ci' of github.com:microsoft/mscclpp i…

d7ad7d1

…nto rjsouza/multinode-ci

Merge branch 'main' into rjsouza/multinode-ci

1730e5a

widen autotuner options

c722034

increase auto for all dsl algorithms

efcea16

Binyang Li (Binyang2014) requested a balanced review from Copilot September 10, 2026 02:55

Copilot started reviewing on behalf of Binyang Li (Binyang2014) September 10, 2026 02:56 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Binyang Li (Binyang2014) reviewed Sep 10, 2026

View reviewed changes

		reduced = sum(values.astype(cp.float32).reshape(-1)[start : start + chunk] for values in encoded_inputs)
		return _encode_reduced_output(case, reduced), None

		# DSL algorithms execute through the Executor and fail without one; native algorithms
		# ignore it, so it is safe to pass unconditionally in run().

Conversation

RJ Souza (Empyreus) commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azure-pipelines Bot commented Aug 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

azure-pipelines Bot commented Aug 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (3)

Uh oh!

Binyang Li (Binyang2014) left a comment

Choose a reason for hiding this comment

Uh oh!

Binyang Li (Binyang2014) Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

Binyang Li (Binyang2014) Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

Binyang Li (Binyang2014) Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

Binyang Li (Binyang2014) Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

Binyang Li (Binyang2014) Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

RJ Souza (Empyreus) commented Aug 25, 2026 •

edited

Loading