Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
6c9e7a5 to
c4edf6b
Compare
7f284cc to
99ed46b
Compare
nvpohanh
left a comment
There was a problem hiding this comment.
[by Codex] Review comments are inline.
| ) | ||
| # A TP1 shared expert is replicated, so compute it before the gather and | ||
| # add it after the reducing combine. | ||
| do_shared_local = getattr( |
There was a problem hiding this comment.
[by Codex] Severity: style | Confidence: High
Please avoid both dynamic getattr probes in this modified expression. DeepseekV4DecoderLayer.mlp is a DeepseekV2MoE, so expose typed state—for example, initialize its optional shared_experts member to None and read self.mlp.shared_experts and self.mlp._shared_expert_tp1 directly.
| @@ -2942,9 +2972,13 @@ def _can_run_tbo(self, forward_batch: ForwardBatch) -> bool: | |||
| ) | |||
| else: | |||
| path_ok = ( | |||
There was a problem hiding this comment.
[by Codex] Severity: style | Confidence: High
Black reformats this expression, and the PR's lint check currently fails. Please apply the repository formatter and commit the resulting flattening of this boolean expression.
99ed46b to
27b9899
Compare
27b9899 to
494ac92
Compare
494ac92 to
3cfc6ee
Compare
|
/tag-and-rerun-ci |
|
/tag-and-rerun-ci |
|
/rerun-failed-ci |
1 similar comment
|
/rerun-failed-ci |
Motivation
Depends on #31700.
DeepSeek-V4 Two-Batch Overlap (TBO) with DP attention,
moe_a2a_backend=none, and attention tensor parallelism greater than oneapplies full-TP variable-length collectives to tensors replicated within each
attention-TP group. Treating those replicas as independent shards corrupts the
gather and combine layouts and produces invalid model output.
This change gives each full-TP rank one shard of its replicated DP-local tensor
and reconstructs the DP-local result across the attention-TP group after the
combine.
Modifications
TBO child batch.
the full-TP variable-length gather.
all-gather within the attention-TP group to reconstruct the replicated
DP-local output.
after the reducing combine.
add focused tests for collective-size mapping and validation.
Accuracy Tests
H100 end-to-end setup
nvidia/DeepSeek-V4-Pro-NVFP4lmsysorg/sglang:nightly-dev-cu13-20260801-e4c4faf8noneOn host 0:
On host 1:
Before: failure reproduction
The failure was reproduced both on pristine upstream
574ead753and on theparent of this change,
68d70aa8f. Both reached the real two-chunk TBO pathand returned HTTP 200 responses with corrupted generated text.
The following server command was run on both hosts:
This exact probe and assertion were run after the server became ready:
Pristine upstream produced:
grepexited 1: FAIL.The parent of this change produced:
grepexited 1: FAIL.The parent run was Slurm job
5616864and also recorded 48 rank-levelis_enable_two_chunk=Trueevents.The pristine-upstream run recorded 48 rank-level
is_enable_two_chunk=Trueevents, representing three global two-chunkforwards. Slurm job
5618929completed0:0; completion means the harnesssuccessfully captured the expected failure.
After: patched result
Source
6c9e7a54awas run with the same model, container, hosts, and topology.The following server command was run on both hosts:
This exact probe and assertion were then run:
Patched output:
grepexited 0: PASS.The following multi-answer check was run twice so both DP routes served it:
Both runs produced:
All three assertions exited 0: PASS.
The sky/seasons check also passed on both DP routes: its assertions found
blue,scatter,season, andtilt.The full GSM8K sweep used:
Runtime coverage and stability:
is_enable_two_chunk=Truerank records5616859:COMPLETED 0:0Focused collective-size tests passed 3/3 in the matching container (Slurm job
5618770,COMPLETED 0:0).Speed Tests and Profiling
The 1,319-request GSM8K API phase completed in 9m01s at concurrency 16
(2.44 completed requests/s). The complete job, including checkpoint staging,
weight loading, JIT compilation, CUDA-graph capture, deterministic checks, and
GSM8K, completed in 26m03s.
The control configurations produce invalid output, so their generated-token
throughput is not a correctness-preserving performance baseline.
Checklist
CI States
Latest PR Test (Base): 🚫 Run #34951900568
Latest PR Test (Extra): ❌ Run #34951900219
Latest PR Test (AMD ROCm 10): ➖ No AMD PR run found for this commit.