Skip to content

Fix ring all-reduce in-place corruption at gpu_num ≥ 3#253

Merged
chhwang merged 14 commits into
mainfrom
pr-c-allreduce-inplace-fix
Jun 10, 2026
Merged

Fix ring all-reduce in-place corruption at gpu_num ≥ 3#253
chhwang merged 14 commits into
mainfrom
pr-c-allreduce-inplace-fix

Conversation

@chhwang

@chhwang chhwang commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Fix ring all-reduce in-place corruption at gpu_num ≥ 3

In-place ring all-reduce mutates input before the next iteration's
send when output aliases input. At TP=2 the single iteration hides
the bug; at gpu_num ≥ 3 every subsequent send carries stale data.

Fix: detect buffer aliasing (output.ref()->buffer()->id() == input.ref()->buffer()->id()) and copy input into a private tensor
before the ring loop.

Also sorted includes in ops_all_reduce_test.cpp to satisfy
clang-format.

ark-dev added 2 commits June 9, 2026 01:31
…pying input into a private buffer before the loop.
…ct buffer aliasing and copy input into a private tensor before the ring loop.
@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.78%. Comparing base (75de9b4) to head (8d4625d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #253      +/-   ##
==========================================
- Coverage   86.88%   86.78%   -0.10%     
==========================================
  Files         125      127       +2     
  Lines        6113     5993     -120     
==========================================
- Hits         5311     5201     -110     
+ Misses        802      792      -10     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ark-dev added 2 commits June 9, 2026 04:41
… UNITTEST_SKIP macro (trailing whitespace) to pass PR #253 linter CI.
…(trailing whitespace / misaligned backslash in UNITTEST_SKIP macro) to pass the linters check on PR #253.
@chhwang chhwang changed the title ark-dev: Fix ring all-reduce in-place corruption at gpu_num ≥ 3: detect buffer aliasing and copy input into a private tensor before the ring loop. Fix ring all-reduce in-place corruption and add GPU-count skip infrastructure Jun 9, 2026
chhwang and others added 6 commits June 9, 2026 09:32
Resolve conflict in ark/ops/ops_test_common.cpp: keep both
ARK_UT_CORRECTNESS env-var gating (#251) and GPU-count skip guard.
…lace-fix) against main; conflict is in ark/ops/ops_test_common.cpp — preserve both P2's UT-gating logic and P4's GPU-count skip logic.
- ops_test_common.cpp:63: join LOG args onto one line (fits in 80 cols)
- unittest_utils.h:248-253: re-align backslashes in UNITTEST_SKIP macro
…e-inplace-fix): ops_test_common.cpp:63 and unittest_utils.h:248-253.
@chhwang chhwang changed the title Fix ring all-reduce in-place corruption and add GPU-count skip infrastructure Fix in-place ring all-reduce corruption at gpu_num ≥ 3 Jun 9, 2026
chhwang and others added 4 commits June 10, 2026 00:53
…esolving any conflicts; cause: BEHIND (behind_by=1).
…esolving any conflicts; cause: BEHIND (behind_by=1).
Alphabetize #include directives to satisfy Google-style clang-format.
Fixes the linters CI failure at line 4:1.
@chhwang chhwang changed the title Fix in-place ring all-reduce corruption at gpu_num ≥ 3 Fix ring all-reduce in-place corruption at gpu_num ≥ 3 Jun 10, 2026
@chhwang
chhwang merged commit c45e366 into main Jun 10, 2026
10 of 11 checks passed
@chhwang
chhwang deleted the pr-c-allreduce-inplace-fix branch June 10, 2026 03:56
chhwang added a commit that referenced this pull request Jun 10, 2026
Sync with main to incorporate #252 (monolithic LayerNorm/Softmax ops)
and #253 (ring allreduce in-place fix). Clean merge, no conflicts.
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