Skip to content

Sync EAGLE verify results across TP ranks - #29003

Open
zihaomu wants to merge 2 commits into
sgl-project:mainfrom
zihaomu:fix-eagle-verify-tp-broadcast-28815
Open

zihaomu wants to merge 2 commits into
sgl-project:mainfrom
zihaomu:fix-eagle-verify-tp-broadcast-28815

Conversation

@zihaomu

@zihaomu zihaomu commented Jun 23, 2026

Copy link
Copy Markdown

Summary

  • Move EAGLE verify result synchronization out of the sampling-only branch so greedy/HIP verify results are also broadcast across TP ranks.
  • Keep predict, accept_index, and num_correct_drafts aligned before later running-batch updates and collectives.
  • Add a CPU regression test that exercises the HIP-forced greedy path with a mocked TP group.

Fixes #28815.

Motivation

On ROCm, EAGLE/NEXTN verify enters the greedy path via _is_hip. The sampling path already broadcasts verify outputs across TP ranks, but the greedy/HIP path returned local verify results. If ranks accept different numbers of draft tokens, later shape-dependent collectives can diverge and hang.

The fix treats TP synchronization as a verify-output invariant rather than a sampling-only behavior.

Testing

  • python3 -m pytest test/registered/unit/spec/test_eagle_verify_tp_sync.py -q
  • pre-commit run --files python/sglang/srt/speculative/eagle_utils.py test/registered/unit/spec/test_eagle_verify_tp_sync.py
  • MI300X/gfx942 ROCm container synthetic 2-rank validation: python3 repro_issue_28815_tp_divergence.py --mode repo-fixed --master-port 29622 --join-timeout-seconds 30

CI States

Latest PR Test (Base): ❌ Run #28007911393
Latest PR Test (Extra): ❌ Run #28007911148

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@ronhuafeng

Copy link
Copy Markdown
Contributor

I did an independent CPU-side validation of this PR head (f98b8edc56).

Checks run:

git diff --check 067987d86^..067987d86
python3 -m py_compile python/sglang/srt/speculative/eagle_utils.py test/registered/unit/spec/test_eagle_verify_tp_sync.py
docker run --rm -v $PWD:/workspace -w /workspace lmsysorg/sglang:dev \
  bash -lc 'PYTHONPATH=python python3 -m pytest test/registered/unit/spec/test_eagle_verify_tp_sync.py -q'

Result: the focused test passed (1 passed). The patch moves the TP broadcast to the common verify-output path, so it covers the _is_hip/greedy branch described in #28815 instead of only the sampling branch. I did not run the full MI300X EAGLE/NEXTN workload here, but the added regression test matches the reported missing-broadcast invariant.

Also checked CI status: lint passed; the current red PR-test entries are gate/skip finish jobs rather than a focused lint/test failure.

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.

Speculative EAGLE verify: greedy/HIP branch missing TP broadcast -> rank divergence -> collective deadlock on ROCm

2 participants