mudler / mudler/vllm.cpp

Half the tie-stability device sweep is a duplicate: VT_MOE_ROUTER_WARP does not change the dispatch above E = 256

Open
#2,604 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
423
Forks
53
Avg merge
20h 26m
Merged PRs (30d)
310

Description

Row: MODEL-MM-QWEN4-EXP

The gap

tests/vt/test_moe_router_tie_stability.cpp (landed by #2595) runs its device
sweep twice, once with VT_MOE_ROUTER_WARP pinned "1" and once pinned "0",
across E in {256, 512, 1024}. At E = 256 those are two different kernels. At
E = 512 and E = 1024 they are the same kernel launched twice:

  • MoeRouterWarpValuesPerThread (src/vt/cuda/moe_router_warp.h:96-98) returns
    0 for any E outside {32, 64, 128, 256}.
  • LaunchRouterWarp (src/vt/cuda/cuda_moe.cu:564) returns false on vpt == 0
    before touching the tensors, so LaunchRouter (:599-609) falls through to
    MoeRouterTopKKernel<Tin,false> whatever the env flag says.

What is and is not affected

The counts are honest. 108 rows, 4652 assertions and the mutant's 104
failures are all real assertions that really executed, and the closed-form
expectation is re-checked each time. Nothing is inflated in the sense of being
uncounted.

The COVERAGE at E > 256 is half what the doubled numbers imply. Of the 108
rows in case 1, 72 are at E in {512, 1024} and 36 of those are byte-identical
repeats of the other 36. In the mutation table the per-cell figure 12 is
2 arms x 3 h x 2 dtypes, so 6 of each 12 are the same disagreement counted a
second time. A reader who takes "both VT_MOE_ROUTER_WARP arms" as two
independent structures at the geometry qwen4_exp actually routes reads twice
the coverage that exists.

A comment saying so is added to the file by #2595. This issue owns the fix.

What closing this means

One of:

  1. Restrict the arm loop to the geometries where the flag changes the dispatch
    (E = 256 here), and say in the file that E > 256 has one kernel; or
  2. Keep both arms and assert the redundancy instead of implying independence —
    e.g. REQUIRE(MoeRouterWarpValuesPerThread(e) == 0) at E > 256, which
    turns the duplicate into a checked property of the dispatch rather than an
    unstated one; or
  3. Widen MoeRouterWarpValuesPerThread so the warp kernel really does admit
    E = 512, which is a kernel change with its own derivation obligation
    (moe_router_warp.h excludes E > 256 on purpose, because the seed loops at
    cuda_moe.cu:71,83 accumulate several experts per thread in ascending order).

(3) is a product change and needs its own spec. (1) or (2) is a test repair and
needs GPU time to re-measure the counts every record cites, which is why it is
not folded into #2595.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with tests/vt/test_moe_router_tie_stability.cpp and trace its arm loop through LaunchRouter in src/vt/cuda/cuda_moe.cu and MoeRouterWarpValuesPerThread in src/vt/cuda/moe_router_warp.h. Choose the test-only repair described in the issue, then rerun the GPU sweep and verify that the recorded rows, assertions, and mutation counts reflect independent coverage or explicitly checked redundancy.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend, testing-qa
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.