Skip to content

feat(moe): register mixed-qtype decode tables for hot-only storage - #703

Open
maikzz32 wants to merge 3 commits into
Luce-Org:mainfrom
maikzz32:upstream-pr/mix-tables-hot-only
Open

maikzz32 wants to merge 3 commits into
Luce-Org:mainfrom
maikzz32:upstream-pr/mix-tables-hot-only

Conversation

@maikzz32

@maikzz32 maikzz32 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Stacked on #701, which adds MoeHybridColdBackend::None. Please review that one first — the diff against main shown here contains both. The change belonging to this PR is the last commit, 16 lines in deepseek4_loader.cpp.

Adaptive (mixed ROCmFPX) experts keep their codebooks out of band and need a decode table registered per resident tensor. register_deepseek4_moe_hybrid_mix_tables demanded a materialized GPU cold owner, so an owner using cold-owner None — whose resident set is exactly its hot experts — was refused.

That refusal was unnecessary. The registrar already handles this shape: ds4_register_compact_mix_tensor returns success for a null tensor whose expert-id list is empty, which is precisely how an absent cold owner presents itself. Only the guard was too narrow. It now accepts either a materialized GPU cold owner, as before, or hot-only storage with no cold owner; a Cpu cold owner is still refused, and nothing else changes.

This also resolves the TODO(cluster-verify) I left beside ds4_cluster_has_mix_experts in my own tree.

Why it matters

It is what makes an adaptive artifact usable by an owner whose cold set lives outside the process. Measured downstream on two Radeon 8060S nodes under ROCm 10 with DeepSeek V4 Flash, each node holding half the routed experts with cold owner None: the adaptive artifact loads where it previously failed with mixed expert qtypes require materialized GPU owners.

Against the uniform artifact on the same two nodes, verify width 4, output byte-identical to a single node:

artifact benchmark prompt free prompt exact-copy fidelity
ROCMFP2-STRIX (uniform) 38.4 tok/s 17.2 tok/s 12/60
ROCMFPX-MIX-STRIX (adaptive) 42.3 tok/s 20.6 tok/s 60/60

Fidelity is the exact-copy protocol from server/docs/DS4.md — 20 identifiers × 3 repeats at temperature 0. The adaptive artifact is not merely faster here; it is the one that answers correctly.

Verification

Built against this PR's base with -DDFLASH27B_GPU_BACKEND=hip -DDFLASH27B_HIP_ARCHITECTURES=gfx1151 -DDFLASH27B_ROCMFP2_AFFINE=ON -DGGML_HIP_GRAPHS=ON on a gfx1151 host: 379/379, no new warnings.

🤖 Generated with Claude Code

Review in cubic

maikzz32 and others added 3 commits September 4, 2026 12:06
MoeHybridColdBackend has Cpu and Gpu: the experts a rank does not hold
hot are evaluated somewhere in this process. There is no way to say "the
non-resident experts are not mine at all — their contribution arrives
from outside". Expert-parallel inference across processes needs exactly
that: each owner evaluates its own experts and a reduction outside ggml
sums the partials.

Adds MoeHybridColdBackend::None:

* storage allocates no cold buffers and never materializes or swaps a
  cold expert;
* the evaluators build no cold graph and never fall back to CPU or
  streamed evaluation. A route whose id was masked to -1 contributes
  zero instead of being routed to a cold owner.

Purely additive: the enum grows a third value that nothing selects by
default, and every Cpu/Gpu path is untouched. Verified by running the
existing single-node hybrid configurations unchanged.

Also adds eval_moe_shared_expert_batched, the piece such an owner needs
that no existing entry point provides: the shared expert alone, batched.
An owner that reduces across processes has to keep the shared term OUT
of the reduced partial (it is replicated, so summing it would count it
once per owner) and add it afterwards, which means evaluating it without
the routed experts.

Used downstream to run DeepSeek V4 Flash expert-parallel across two
hosts, each holding half the routed experts.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…a time

An owner with cold-owner None holds a REDUCED expert stack, and
mmq_full_batch_ok is false for a reduced stack because MMQ's mul_mat_id
illegal-accesses on it. eval_moe_hybrid_ffn_batched therefore falls into
its sub-batch loop, whose size is

    min(mmq_safe_sub_batch(), moe_hybrid_prefill_hot_sub_batch_limit())

which is 1 on gfx1151, where mmq_safe_sub_batch() returns 1 for
compute < sm_80. A 1517-token prefill then runs 1517 x 43 = 65k graph
computes.

With cold-owner None there is exactly one owner and every route that
survives masking is resident, so the whole batch can take the packing
the heterogeneous prefill already uses: one expert-major graph per
layer, via eval_moe_owner_expert_major_batched. That path was written
for a reduced stack in the first place, which is why it does not need
the MMQ full-batch guarantee.

Measured on two Ryzen AI Max 395 (Radeon 8060S, gfx1151, ROCm 10),
DeepSeek V4 Flash, --ds4-prefill sparse --chunk 2048, same binary and
clocks, prefill wall time:

    1517-token prompt:   34.1 s -> 11.45 s   (FFN 25.9 s -> 3.7 s)
    12017-token prompt: 283.9 s -> 96.9 s

Output parity: the completion is unchanged by this patch, and the
128-token greedy benchmark stays byte-identical to a single-node run of
the same binary (sha256 87964cbd...), 3 runs.

Reachable only with cold-owner None, so no existing configuration
changes behaviour. On sm_80+ the sub-batch is 8 rather than 1, so the
same shape would have been 8x less bad but still 8k graph computes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Stacked on Luce-Org#701, which adds MoeHybridColdBackend::None. Review that one
first; the diff against main shows both.

Adaptive (mixed ROCmFPX) experts keep their codebooks out of band and need a
decode table registered per resident tensor.
register_deepseek4_moe_hybrid_mix_tables demanded a materialized GPU cold
owner, so an owner using cold-owner None -- whose resident set is exactly its
hot experts -- was refused, even though that is the storage shape the
registrar already handles: ds4_register_compact_mix_tensor returns success
for a null tensor whose expert-id list is empty, which is how an absent cold
owner presents itself. Only the guard was too narrow.

It now accepts either a materialized GPU cold owner, as before, or hot-only
storage with no cold owner. Nothing else changes, and a Cpu cold owner is
still refused.

This is what makes an adaptive artifact usable by an owner whose cold set
lives outside the process. Measured downstream on two Radeon 8060S nodes
under ROCm 10 with DeepSeek V4 Flash, where each node holds half the routed
experts with cold owner None: the adaptive ROCMFPX-MIX artifact loads where
it previously failed, and against the uniform ROCMFP2 artifact it measures
42.3 against 38.4 tok/s at a verify width of 4, 20.6 against 17.2 on a
free-form prompt, and 60/60 against 12/60 on the exact-copy fidelity check
from server/docs/DS4.md. Output is byte-identical to a single node.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
maikzz32 added a commit to maikzz32/lucebox-halo-cluster that referenced this pull request Sep 4, 2026
…g#703

Luce-Org#702 is independent. Luce-Org#703 is stacked on Luce-Org#701 because it needs
MoeHybridColdBackend::None, which is not upstream yet.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai 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.

3 issues found across 5 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="server/src/deepseek4/deepseek4_loader.cpp">

<violation number="1" location="server/src/deepseek4/deepseek4_loader.cpp:1357">
P1: The new `None` branch is unreachable from DeepSeek4 initialization, so hot-only mixed artifacts still never reach this registration path. Wire the hot-only/cluster configuration to set `cold_expert_backend = MoeHybridColdBackend::None` and invoke this registration for that mode; otherwise this change does not enable the advertised external-cold-owner flow.</violation>
</file>

<file name="server/src/common/moe_hybrid_ffn_eval.cpp">

<violation number="1" location="server/src/common/moe_hybrid_ffn_eval.cpp:4292">
P3: `eval_moe_shared_expert_batched` is exported here but has no call site anywhere in the repository, so this cluster shared-expert implementation never runs. Wire it into the cluster all-reduce path or remove it and its declaration.</violation>

<violation number="2" location="server/src/common/moe_hybrid_ffn_eval.cpp:4302">
P2: When `n_tokens` is negative, this cast occurs before the `n_tokens <= 0` guard, causing a huge allocation request. Check `n_tokens` before converting it to `size_t`.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

// precisely how an absent cold owner presents itself. Only this check stood
// in the way.
const bool hot_only =
storage.cold_backend_kind == MoeHybridColdBackend::None &&

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.

P1: The new None branch is unreachable from DeepSeek4 initialization, so hot-only mixed artifacts still never reach this registration path. Wire the hot-only/cluster configuration to set cold_expert_backend = MoeHybridColdBackend::None and invoke this registration for that mode; otherwise this change does not enable the advertised external-cold-owner flow.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/deepseek4/deepseek4_loader.cpp, line 1357:

<comment>The new `None` branch is unreachable from DeepSeek4 initialization, so hot-only mixed artifacts still never reach this registration path. Wire the hot-only/cluster configuration to set `cold_expert_backend = MoeHybridColdBackend::None` and invoke this registration for that mode; otherwise this change does not enable the advertised external-cold-owner flow.</comment>

<file context>
@@ -1346,11 +1346,24 @@ bool register_deepseek4_moe_hybrid_mix_tables(
+    // precisely how an absent cold owner presents itself. Only this check stood
+    // in the way.
+    const bool hot_only =
+        storage.cold_backend_kind == MoeHybridColdBackend::None &&
+        !storage.materialized_cold_experts;
+    const bool gpu_owners =
</file context>

std::vector<float> & out,
std::string * err) {
const int n_embd = cfg.n_embd;
out.assign((size_t)n_embd * (size_t)n_tokens, 0.0f);

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.

P2: When n_tokens is negative, this cast occurs before the n_tokens <= 0 guard, causing a huge allocation request. Check n_tokens before converting it to size_t.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/common/moe_hybrid_ffn_eval.cpp, line 4302:

<comment>When `n_tokens` is negative, this cast occurs before the `n_tokens <= 0` guard, causing a huge allocation request. Check `n_tokens` before converting it to `size_t`.</comment>

<file context>
@@ -4232,4 +4283,73 @@ bool eval_moe_hybrid_ffn_gpu_resident(
+    std::vector<float> &            out,
+    std::string *                   err) {
+    const int n_embd = cfg.n_embd;
+    out.assign((size_t)n_embd * (size_t)n_tokens, 0.0f);
+    if (n_tokens <= 0) return true;
+    if (!desc.ffn_up_shexp || !desc.ffn_gate_shexp || !desc.ffn_down_shexp) {
</file context>
Suggested change
out.assign((size_t)n_embd * (size_t)n_tokens, 0.0f);
if (n_tokens <= 0) {
out.clear();
return true;
}
out.assign((size_t)n_embd * (size_t)n_tokens, 0.0f);

// cleared), all-reduces it, and adds this locally computed term afterwards.
// The graph is cached per n_tokens in storage.shared_batched_graph, which
// release_graph_caches() already frees.
bool eval_moe_shared_expert_batched(

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.

P3: eval_moe_shared_expert_batched is exported here but has no call site anywhere in the repository, so this cluster shared-expert implementation never runs. Wire it into the cluster all-reduce path or remove it and its declaration.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/common/moe_hybrid_ffn_eval.cpp, line 4292:

<comment>`eval_moe_shared_expert_batched` is exported here but has no call site anywhere in the repository, so this cluster shared-expert implementation never runs. Wire it into the cluster all-reduce path or remove it and its declaration.</comment>

<file context>
@@ -4232,4 +4283,73 @@ bool eval_moe_hybrid_ffn_gpu_resident(
+// cleared), all-reduces it, and adds this locally computed term afterwards.
+// The graph is cached per n_tokens in storage.shared_batched_graph, which
+// release_graph_caches() already frees.
+bool eval_moe_shared_expert_batched(
+    ggml_backend_t                  gpu_backend,
+    const MoeHybridConfig &         cfg,
</file context>

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.

1 participant