Skip to content

[2/10] fix(training): synchronize DDP once per accumulation window - #958

Merged
jiapingW merged 4 commits into
mainfrom
codex/pr942-02-ddp-accumulation
Oct 22, 2026
Merged

jiapingW merged 4 commits into
mainfrom
codex/pr942-02-ddp-accumulation

Conversation

@jiapingW

@jiapingW jiapingW commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

DDP accumulated microbatches were still synchronizing because no_sync covered only backward.

Modifications

Expose a backend accumulation context and enter it around both forward and backward. Keep FSDP backward deferral, and add optional positive training.ddp_bucket_cap_mb configuration. A two-rank test checks collective counts and optimizer-weight parity.

Stack and dependencies

Layer 2/10 split from #942. Previous layer: #957 (codex/pr942-01-streaming-reads). Review the diff against this PR's base branch; it contains only this layer. The full stack index is maintained in #942.

Actual prerequisites: No earlier feature is required; this layer uses existing main APIs.

The linear stack is the review/merge order. Model-specific optimizations are independently opt-in; the projection-checkpointing and attention layers expose shared capabilities. When merging a lower PR with squash, restack the remaining branches before merging them so already-reviewed commits do not reappear. The repository GPU test workflow only auto-runs for PRs targeting main; upper layers need that CI run when retargeted, while lint runs for all PRs.

Source: #942 at 211b87fc7d707e3a628cb95de5c73340af863c04, split onto main fdcc2eddf1b56b5b24a5a15d3abcb366adfeb5aa. The original branch is preserved.

Accuracy Test

  • This layer at 00f7af287641: 40 tests run, zero failures/errors on H20 with Python 3.11.12 / PyTorch 2.13.0+cu130.
  • Final stack at eaa14f102d6a: 405 integration tests run, zero failures/errors; 2 skipped on H20 with Python 3.11.12 / PyTorch 2.13.0+cu130. Each module runs in a fresh process to isolate process-global distributed state. This is targeted regression coverage, not a full-suite or downstream task-accuracy claim.
  • Repository pre-commit checks passed for the final stack; each layer was formatted with the repository's pinned Black/isort versions.

Layer command (run in this branch's checkout, PYTHONPATH=.):

CUDA_VISIBLE_DEVICES=0 OMP_NUM_THREADS=4 python -m unittest \
  tests.test_training.test_ddp_accumulation \
  tests.test_config.test_schema -v

Qwen3 tokenizer regressions reuse the cached official tokenizer with HF_HUB_OFFLINE=1 and HF_HUB_CACHE pointing to that cache. No training job was launched.

Benchmark & Profiling

No new end-to-end throughput or downstream accuracy benchmark was run for this split. Model-side tests exercise numerical parity and gradients; historical measurements remain in #942 with their original scope and caveats.

Checklist

  • Format changed code.
  • Run the layer's regression tests and final-stack integration tests.
  • Keep feature tests/documentation with their owning layer.
  • Complete GitHub CI and any required downstream accuracy/performance validation before merge.

@jiapingW jiapingW changed the title [2/11] fix(training): synchronize DDP once per accumulation window [2/10] fix(training): synchronize DDP once per accumulation window Oct 14, 2026
yushengsu-thu added a commit to yushengsu-thu/SpecForge that referenced this pull request Oct 15, 2026
… into the MoE 8-expert experiment branch

Brings is_causal (configurable draft attention range/direction), dspark_flatten_projection /
dspark_cache_projection, ddp_bucket_cap_mb, the DDP once-per-window synchronization (sgl-project#958, which
replaces this branch's prepare_micro_step fix; the matching test is dropped), the removal of the
implicit qwen3-instruct system prompt (sgl-project#959), balanced consumer optimizer windows (sgl-project#966) and the
DSpine algorithm. Conflicts in specforge/training/backend.py and controller.py were resolved in
favour of the PR.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
yushengsu-thu added a commit to yushengsu-thu/SpecForge that referenced this pull request Oct 15, 2026
…yers 1-7 of the sgl-project#942 split), matching the dense-1/8 branch

The combined PR sgl-project#942 merge was reverted in the previous commit; this takes the split stack instead
(sgl-project#957 streaming-read fixes, sgl-project#958 DDP sync once per accumulation window, sgl-project#959 no implicit qwen3-instruct
system prompt, sgl-project#960 is_causal, sgl-project#961 fused DFlash head, sgl-project#962 selective checkpoint contexts, sgl-project#963 shared
flatten_vocab_projection / cache_vocab_projection), so the MoE arm carries exactly the same upstream code
as the dense-1/8 arm. Conflicts in specforge/training/backend.py and controller.py resolved in the stack's
favour.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@maocheng23 maocheng23 self-assigned this Oct 21, 2026
@maocheng23

Copy link
Copy Markdown
Collaborator

LGTM, can you please simplify the test file, now it's too long

Base automatically changed from codex/pr942-01-streaming-reads to main October 22, 2026 01:24
@jiapingW
jiapingW merged commit 0c7e80b into main Oct 22, 2026
6 checks passed
@jiapingW
jiapingW deleted the codex/pr942-02-ddp-accumulation branch October 22, 2026 03:18
yushengsu-thu added a commit to yushengsu-thu/SpecForge that referenced this pull request Nov 9, 2026
…l-project#966 (consumer dispatch) onto 475c9b9

Speed-only changes so the dense-6144 run carries the same loss kernels as the dense-1/8 5-epoch run (cde2f05):

- sgl-project#961, fused FP32 CE + teacher L1 at logits level: 158cfdd, b0bd4f6, b525e15
  (training.fused_logits_loss defaults to true, training.objective_checkpointing to true)
- sgl-project#966, cost-balanced consumer dispatch and backpressure fixes: 7aad50d, c124809, c59e3bf,
  70061c0, 4f4c4fa, 1e1a4b9, a607703 (runtime.consumer_dispatch stays round_robin)

Conflict resolutions against this branch: schema.py and examples/configs/README.md take only the
fused_logits_loss lines (no ddp_bucket_cap_mb, which belongs to sgl-project#958); dflash_family_model.py keeps
the MoE hooks _take_moe_aux_loss_and_log and on_optimizer_step next to the renamed
_selector_token_terms; tests/test_config/test_schema.py keeps the tri-state Liger test and drops the
ddp-bucket test.

Measured on the dense-6144 run (8x B200, one capture server + six trainers): train compute
2.39 -> 1.99 s/step; objective benchmark (8192 tokens, checkpointing on) 130.8 -> 74.7 ms per
micro-step with identical loss.
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