Skip to content

Fix DeepSeek V4 routed expert serialization#673

Merged
zhangqi-chen merged 1 commit into
hw-native-sys:mainfrom
wangqin1723-max:fix/dsv4-expert-routed-manual-deps
Jul 2, 2026
Merged

Fix DeepSeek V4 routed expert serialization#673
zhangqi-chen merged 1 commit into
hw-native-sys:mainfrom
wangqin1723-max:fix/dsv4-expert-routed-manual-deps

Conversation

@wangqin1723-max

Copy link
Copy Markdown
Contributor

Summary

  • Move expert_routed tile execution into manual_scope and wire explicit task dependencies for gate/up/act/quant/w2 stages, avoiding conservative serialization across independent local expert tiles.
  • Keep the dynamic n_tiles loop so empty experts do not launch routed work; only non-empty tiles emit downstream marker tasks to make recv_y visible to the following combine stage.
  • Performance notes from L2 swimlane AICore end-to-end timing: 7-layer decode_fwd rank1 improved from 11057.26 us / 10779.18 us prior normal runs to 10240.10 us, and avoids the previously observed 38670.98 us / 46190.78 us serialized cases.
  • Standalone expert_routed.py with the same 25 non-empty tiles is 521.30 us vs 515.96 us before because it adds 25 exp_routed_tile_done marker tasks; this is the expected small marker overhead that buys correct downstream dependencies in the full graph.
  • Validation: python -m py_compile models/deepseek/v4/expert_routed.py; ruff check --config ruff.toml models/deepseek/v4/expert_routed.py; python tests/lint/check_headers.py; python tests/lint/check_english_only.py; standalone expert_routed.py -p a2a3 -d 0 --enable-l2-swimlane PASS with recv_y validation.

Related Issues

None

Move routed expert tile execution into manual scope with explicit task dependencies so independent expert tiles are not serialized by conservative dynamic recv_y writes. Re-register non-empty tile outputs for downstream combine with lightweight marker tasks.
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The expert_routed kernel in models/deepseek/v4/expert_routed.py is restructured to use explicit manual dependency scoping. A new TILES_PER_EXPERT constant sizes a per-expert tile completion tracking array (w2_act_tids), and the loop uses pl.manual_scope(), manual_dep=True tensors, and pl.at deps-based barriers before writing to recv_y_flat, replacing prior conservative serialization.

Changes

Expert Routed Kernel Dependency Rework

Layer / File(s) Summary
Manual dependency scoping and per-tile barriers
models/deepseek/v4/expert_routed.py
Adds TILES_PER_EXPERT constant, introduces w2_act_tids tile completion tracking, wraps per-expert computation in pl.manual_scope() with manual_dep=True tensors and nested pl.spmd(...) regions, and adds a final pl.at(level=CORE_GROUP, deps=[...]) barrier gating writes to recv_y_flat.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant OuterLoop as expert_routed loop
  participant TileCompute as Per-expert tile compute
  participant DepArray as w2_act_tids
  participant RecvY as recv_y_flat

  OuterLoop->>TileCompute: enter pl.manual_scope() for expert tile
  TileCompute->>TileCompute: compute gate/up, SwiGLU, INT8 requant, w2 GEMM
  TileCompute->>DepArray: mark tile completion (manual_dep=True)
  OuterLoop->>DepArray: pl.at(deps=[...]) check tile-done barrier
  DepArray-->>OuterLoop: dependency resolved
  OuterLoop->>RecvY: write/update recv_y_flat slice for tile
Loading

Possibly related PRs

  • hw-native-sys/pypto-lib#445: Both PRs modify the same expert_routed w2/tiling loop structure around RECV_TILE-sized tiles and TILES_PER_EXPERT-related scheduling.
  • hw-native-sys/pypto-lib#594: Both PRs restructure the per-expert/per-tile execution flow around recv_y_flat writes and tiled gate/up/w2 stages.

Poem

A rabbit hops through tiles so fine,
Each dependency drawn in line,
No more waiting, serialized and slow,
Now barriers guard where writes should go,
recv_y_flat, safe and bright—
Hop, hop, hooray, the kernel's right! 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title concisely and accurately summarizes the main change: fixing DeepSeek V4 routed expert serialization.
Description check ✅ Passed The description clearly matches the routed-expert serialization fix and its validation/performance notes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces manual scoping and explicit task dependency tracking in the expert_routed function to optimize parallel execution and prevent conservative serialization of dynamic writes across experts. Specifically, it wraps the expert routing stages in a pl.manual_scope() block, tracks task IDs for the activation stages, and uses a subsequent loop with pl.at to synchronize on the completed tiles. There are no review comments, so we have no additional feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@models/deepseek/v4/expert_routed.py`:
- Line 46: `TILES_PER_EXPERT` is using floor division while the routing code
computes `n_tiles` with ceil division, which can make `w2_act_tids` too small
when `RECV_MAX` is not divisible by `RECV_TILE`. Update the `TILES_PER_EXPERT`
definition in `expert_routed.py` to use the same ceil-based tile count as the
loop, or add a hard divisibility assertion near the `RECV_MAX`/`RECV_TILE`
constants and ensure the same contract is enforced in the related routing logic
(`w2_act_tids`, `n_tiles`, and the tile loop sites).
- Around line 190-199: The final write in the W2 path can reintroduce nonzero
values in padded rows because `recv_weights` is applied after invalid rows were
already zeroed; update the `expert_routed.py` logic around `row_scale_blk`,
`w2_scale_chunk`, and the `recv_y_flat` assignment so `y_2d` is re-masked after
the last scaling step and before casting/writing. Keep the zeroing aligned with
the final tile write in the `pl.pipeline(W2_ACT_INNER, stage=2)` loop so
`recv_y[e, cnt[e]:, :]` remains zero even if padding-row weights are
uninitialized or NaN.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4f045e2a-138f-4c26-bd82-dc96f1070344

📥 Commits

Reviewing files that changed from the base of the PR and between 1b9a997 and 83d818b.

📒 Files selected for processing (1)
  • models/deepseek/v4/expert_routed.py

D_OUT_TILE_ACT = 512
W2_INNER = 4
W2_ACT_INNER = 8
TILES_PER_EXPERT = RECV_MAX // RECV_TILE

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Make TILES_PER_EXPERT match the ceil-tile loop.

n_tiles is computed with ceiling division, but TILES_PER_EXPERT uses floor division. If RECV_MAX is ever not divisible by RECV_TILE, w2_act_tids[tile_idx] can index past the allocated task-id array. Use ceiling division or add a static divisibility guard.

Proposed fix
-TILES_PER_EXPERT = RECV_MAX // RECV_TILE
+TILES_PER_EXPERT = (RECV_MAX + RECV_TILE - 1) // RECV_TILE

Or, if divisibility is required by the kernel contract:

+assert RECV_MAX % RECV_TILE == 0
 TILES_PER_EXPERT = RECV_MAX // RECV_TILE

Also applies to: 75-79, 207-210

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@models/deepseek/v4/expert_routed.py` at line 46, `TILES_PER_EXPERT` is using
floor division while the routing code computes `n_tiles` with ceil division,
which can make `w2_act_tids` too small when `RECV_MAX` is not divisible by
`RECV_TILE`. Update the `TILES_PER_EXPERT` definition in `expert_routed.py` to
use the same ceil-based tile count as the loop, or add a hard divisibility
assertion near the `RECV_MAX`/`RECV_TILE` constants and ensure the same contract
is enforced in the related routing logic (`w2_act_tids`, `n_tiles`, and the tile
loop sites).

Comment on lines +190 to +199
row_scale_blk = pl.mul(h_tile_scale_dq, w_col_blk)
for dg in pl.pipeline(W2_ACT_INNER, stage=2):
d0 = d_base + dg * D_OUT_TILE_ACT
y_2d_i32 = y_i32[:, d0 : d0 + D_OUT_TILE_ACT]
w2_scale_chunk = routed_w2_scale[local_i : local_i + 1, d0 : d0 + D_OUT_TILE_ACT]
y_2d = pl.cast(y_2d_i32, target_type=pl.FP32, mode="none")
y_2d = pl.col_expand_mul(pl.row_expand_mul(y_2d, row_scale_blk), w2_scale_chunk)
recv_y_flat[flat_t0 : flat_t0 + RECV_TILE, d0 : d0 + D_OUT_TILE_ACT] = pl.cast(
y_2d, target_type=pl.BF16, mode="rint"
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Re-mask the final tile before writing recv_y_flat.

Invalid rows are zeroed before W2, but recv_weights is applied afterward. If padding-row weights are uninitialized/NaN, the full-tile write can violate the reference contract that recv_y[e, cnt[e]:, :] stays zero. Mask y_2d after the final scaling step.

Proposed fix
                     for dg in pl.pipeline(W2_ACT_INNER, stage=2):
                         d0 = d_base + dg * D_OUT_TILE_ACT
                         y_2d_i32 = y_i32[:, d0 : d0 + D_OUT_TILE_ACT]
                         w2_scale_chunk = routed_w2_scale[local_i : local_i + 1, d0 : d0 + D_OUT_TILE_ACT]
                         y_2d = pl.cast(y_2d_i32, target_type=pl.FP32, mode="none")
                         y_2d = pl.col_expand_mul(pl.row_expand_mul(y_2d, row_scale_blk), w2_scale_chunk)
+                        y_2d = pl.fillpad(
+                            pl.set_validshape(y_2d, valid_rows, D_OUT_TILE_ACT),
+                            pad_value=pl.PadValue.zero,
+                        )
                         recv_y_flat[flat_t0 : flat_t0 + RECV_TILE, d0 : d0 + D_OUT_TILE_ACT] = pl.cast(
                             y_2d, target_type=pl.BF16, mode="rint"
                         )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
row_scale_blk = pl.mul(h_tile_scale_dq, w_col_blk)
for dg in pl.pipeline(W2_ACT_INNER, stage=2):
d0 = d_base + dg * D_OUT_TILE_ACT
y_2d_i32 = y_i32[:, d0 : d0 + D_OUT_TILE_ACT]
w2_scale_chunk = routed_w2_scale[local_i : local_i + 1, d0 : d0 + D_OUT_TILE_ACT]
y_2d = pl.cast(y_2d_i32, target_type=pl.FP32, mode="none")
y_2d = pl.col_expand_mul(pl.row_expand_mul(y_2d, row_scale_blk), w2_scale_chunk)
recv_y_flat[flat_t0 : flat_t0 + RECV_TILE, d0 : d0 + D_OUT_TILE_ACT] = pl.cast(
y_2d, target_type=pl.BF16, mode="rint"
)
row_scale_blk = pl.mul(h_tile_scale_dq, w_col_blk)
for dg in pl.pipeline(W2_ACT_INNER, stage=2):
d0 = d_base + dg * D_OUT_TILE_ACT
y_2d_i32 = y_i32[:, d0 : d0 + D_OUT_TILE_ACT]
w2_scale_chunk = routed_w2_scale[local_i_i : local_i + 1, d0 : d0 + D_OUT_TILE_ACT]
y_2d = pl.cast(y_2d_i32, target_type=pl.FP32, mode="none")
y_2d = pl.col_expand_mul(pl.row_expand_mul(y_2d, row_scale_blk), w2_scale_chunk)
y_2d = pl.fillpad(
pl.set_validshape(y_2d, valid_rows, D_OUT_TILE_ACT),
pad_value=pl.PadValue.zero,
)
recv_y_flat[flat_t0 : flat_t0 + RECV_TILE, d0 : d0 + D_OUT_TILE_ACT] = pl.cast(
y_2d, target_type=pl.BF16, mode="rint"
)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@models/deepseek/v4/expert_routed.py` around lines 190 - 199, The final write
in the W2 path can reintroduce nonzero values in padded rows because
`recv_weights` is applied after invalid rows were already zeroed; update the
`expert_routed.py` logic around `row_scale_blk`, `w2_scale_chunk`, and the
`recv_y_flat` assignment so `y_2d` is re-masked after the last scaling step and
before casting/writing. Keep the zeroing aligned with the final tile write in
the `pl.pipeline(W2_ACT_INNER, stage=2)` loop so `recv_y[e, cnt[e]:, :]` remains
zero even if padding-row weights are uninitialized or NaN.

@zhangqi-chen zhangqi-chen merged commit 98271e1 into hw-native-sys:main Jul 2, 2026
5 of 7 checks passed
zhangqi-chen pushed a commit that referenced this pull request Jul 3, 2026
## Summary

- replace the routed expert marker-task bridge with auto-scope W2
epilogue block assembly
- keep gate/up/activation/W2 matmul in manual scope with explicit
dependencies
- write static W2 epilogue channel blocks through pl.assemble to avoid
exp_w2_act_1/2 self-dependency chains

## Validation

- python -m py_compile models/deepseek/v4/expert_routed.py
- ruff check models/deepseek/v4/expert_routed.py
- python models/deepseek/v4/decode_fwd.py -p a2a3 --ep 2 -d 0,1
--enable-l2-swimlane --compile-only
- Full device run before rebasing onto current main:
build_output/_jit_l3_decode_fwd_20260703_111753, PASS; exp_w2_act_2
self-chain edges dropped to 0 on rank0/rank1 swimlane traces.

PR #673 is already merged, so this follow-up PR carries only the
additional W2 epilogue assembly change on top of current main.
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