Skip to content

decode_cluster_parity: the graph replays wait for their uploads (the test failed now and then) - #536

Closed
sergqwer wants to merge 1 commit into
Niko1221:mainfrom
sergqwer:pr/cluster-parity-uploads
Closed

sergqwer wants to merge 1 commit into
Niko1221:mainfrom
sergqwer:pr/cluster-parity-uploads

Conversation

@sergqwer

@sergqwer sergqwer commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

decode_cluster_parity fails now and then on its own: a graph replay reports one differing token, from run to run, with no code change.

Cause: run_graph_case uploads each replay's scores, steps and logits with Dev::put, a pageable cudaMemcpy on the legacy stream. It then launches the graph on cs, a cudaStreamNonBlocking stream.

  • A pageable host-to-device cudaMemcpy can return once the data is staged, before its DMA has landed.
  • A non-blocking stream does not wait for the legacy stream.

So the captured cluster kernels sometimes read the previous replay's buffers. The one-CTA reference runs later on the same stream, after the copy has landed, and the two disagree. This is the same race 0.1.36 fixed in prefill_fused_moe_test ("the fused test waits for its uploads"). The test's other cases are not affected: the plain cases run on the legacy stream, ordered after the copy, and the timing cases compare nothing.

Fix: cudaDeviceSynchronize() after the uploads, before cudaGraphLaunch, as in the fused test. Test only; no engine code changes. The engine itself feeds these kernels from kernels on the same stream, not from host uploads, so the cluster kernels' bit-identity holds there.

Measured (RTX 5090, Windows 11, CUDA 13.3, driver 617.14; the test built from v0.1.37):

runs that failed graph replays that differed
v0.1.37 as is, first 20 runs 4 of 20 4 of 120
v0.1.37 as is, 20 runs alternating with the next row 6 of 20 9 of 120
with this change, alternating 0 of 20 0 of 120
this branch, 20 more runs 0 of 20 0 of 120

Every failure was a graph replay with one differing token. The QSA top-k cases (434) and the argmax cases (216) passed in all runs.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VZy1yKaDDiA8a7svdwaHio

The test uploads each replay's scores and logits with a pageable cudaMemcpy (legacy stream) and replays the graph on a
non-blocking stream. A pageable copy can return before its DMA has landed, and the non-blocking stream does not wait
for the legacy one, so now and then the cluster kernels read the previous replay's logits: 0-2 of the 6 replays
reported one differing token, run to run on an RTX 5090. A device sync after the uploads, as 0.1.36 did for the fused
test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VZy1yKaDDiA8a7svdwaHio
Niko1221 added a commit that referenced this pull request Oct 3, 2026
… and timings use it (#548, as #536)

Dev::put uploads with a pageable cudaMemcpy, which can return before its DMA lands, and the graph replays and the
timing cases run on a cudaStreamNonBlocking stream that does not wait for the legacy stream: the graph could read
the previous replay's logits (one argmax token off; 5 of 6 replays on an RTX 5060 Ti, now and then on a 5090).
Dev::put now synchronizes the device after its copy, which covers run_graph_case and bench(). Test only; no engine
code.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0154bwiad8gY7wRehCXy27pN
@Niko1221

Niko1221 commented Oct 3, 2026

Copy link
Copy Markdown
Owner

Thanks for finding this and for the fix! The same change went into 0.1.38 as part of #548 (decode_cluster_parity waits for every upload before the graph replays), so this PR is superseded. Closing.

@Niko1221 Niko1221 closed this Oct 3, 2026
Niko1221 added a commit that referenced this pull request Oct 6, 2026
… and timings use it (#548, as #536)

Dev::put uploads with a pageable cudaMemcpy, which can return before its DMA lands, and the graph replays and the
timing cases run on a cudaStreamNonBlocking stream that does not wait for the legacy stream: the graph could read
the previous replay's logits (one argmax token off; 5 of 6 replays on an RTX 5060 Ti, now and then on a 5090).
Dev::put now synchronizes the device after its copy, which covers run_graph_case and bench(). Test only; no engine
code.
anon761 pushed a commit to anon761/Strata-upstream that referenced this pull request Oct 6, 2026
… and timings use it (Niko1221#548, as Niko1221#536)

Dev::put uploads with a pageable cudaMemcpy, which can return before its DMA lands, and the graph replays and the
timing cases run on a cudaStreamNonBlocking stream that does not wait for the legacy stream: the graph could read
the previous replay's logits (one argmax token off; 5 of 6 replays on an RTX 5060 Ti, now and then on a 5090).
Dev::put now synchronizes the device after its copy, which covers run_graph_case and bench(). Test only; no engine
code.
anon761 pushed a commit to anon761/Strata-upstream that referenced this pull request Oct 6, 2026
… and timings use it (Niko1221#548, as Niko1221#536)

Dev::put uploads with a pageable cudaMemcpy, which can return before its DMA lands, and the graph replays and the
timing cases run on a cudaStreamNonBlocking stream that does not wait for the legacy stream: the graph could read
the previous replay's logits (one argmax token off; 5 of 6 replays on an RTX 5060 Ti, now and then on a 5090).
Dev::put now synchronizes the device after its copy, which covers run_graph_case and bench(). Test only; no engine
code.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0154bwiad8gY7wRehCXy27pN
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