Skip to content

fix(moe): preserve duplicate gguf expert routes - #852

Closed
zihaomu wants to merge 2 commits into
FlashML-org:mainfrom
zihaomu:fix/ggml-duplicate-expert-routes
Closed

zihaomu wants to merge 2 commits into
FlashML-org:mainfrom
zihaomu:fix/ggml-duplicate-expert-routes

Conversation

@zihaomu

@zihaomu zihaomu commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Related to #671; follows #823.

Problem

For ggml.mul_mat_id with MMQ and repeated nonnegative expert IDs in one token, mm_ids_helper packed only one row per (token, expert) but counted every route when computing expert offsets. The resulting holes in ids_src1 / ids_dst could produce wrong output or an illegal GPU memory access. A direct call with 9 tokens, top-k 3 and IDs [0, 0, 2] for the first token reproduced the fault on an R9700.

#823 fixed the known hybrid-decode trigger by preserving inactive routes as -1. Normal model top-k produces distinct active IDs. This PR makes the lower-level operator safe for callers that supply repeated active IDs; it does not claim an observed production model regression after #823.

Change

Pack one record for every matching route and derive expert bounds from the same route count. The common fixed top-k path uses shared memory when it fits; larger inputs and generic top-k use a direct two-pass mapping. Document the per-route contract and add four CPU-reference regression cases for Q4_0 / IQ1_M and top-k 3 / 8.

Validation

  • ROCm 10.1, Radeon AI PRO R9700 (gfx1201), PyTorch 2.14.0: FREETOKEN_DISABLE_KERNEL_CACHE=1 python -m pytest tests/kernels/test_ggml.py -q -> 65 passed. An external qualification also passed 65 combinations, including repeated active IDs, inactive -1, per-route activations and a large shared-memory fallback.
  • RTX 5090 (sm_120), driver 615.71.09, CUDA / nvcc 13.0.88, PyTorch 2.14.1+cu130, apache-tvm-ffi 0.1.13.post3: the same GGML test command rebuilt the native module for sm_120 -> 65 passed, 0 skipped. The original 9-token IQ1_M shape also matched a CPU reference with shared and per-route activations (3,456 elements each; max absolute errors 4.29e-6 and 5.25e-6).
  • Exploratory R9700 Q4_0/BF16 Graph replay for unique IDs (main -> patch): T=17/top-2 15.86 -> 15.78 us, T=32/top-8 16.01 -> 16.01 us, T=256/top-8 50.18 -> 50.89 us. These are operator measurements, not model throughput.

All tests used synthetic packed GGUF expert tensors; no model checkpoint or end-to-end serving run is claimed.

@github-actions github-actions Bot added the gguf label Nov 3, 2026
@jason-fxz

Copy link
Copy Markdown
Collaborator

No caller produces repeated active ids after #823, so there's no workload for this fix. Closing this.

@jason-fxz jason-fxz closed this Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants