Skip to content

npu: Hy-Memory-30B-A3B (hy_v3) runs on the existing MoE path, read the way its pack states it (Refs #1786) - #2279

Merged
bong-water-water-bong merged 2 commits into
mainfrom
cl-flm/hy-v3
Nov 12, 2026
Merged

bong-water-water-bong merged 2 commits into
mainfrom
cl-flm/hy-v3

Conversation

@bong-water-water-bong

@bong-water-water-bong bong-water-water-bong commented Nov 12, 2026 •

Copy link
Copy Markdown
Collaborator

What

FastFlowLM's Hy-Memory-30B-A3B-NPU2 (Tencent hy_v3, 30B MoE, 128 experts top 8) now answers on the host. Before, it was refused (moe router), and once loaded its reply was of of of of. Refs #1786. Host-side only; the NPU is off.

hy_v3 routes the way DeepSeek-V3 does (our llama.cpp fork's src/models/hy-v3.cpp, build_moe_ffn with sigmoid gating):

  • sigmoid router probabilities;
  • top-k chosen on the probabilities plus an expert bias;
  • weights from the unbiased probabilities, sum-normalised and scaled;
  • one ungated shared expert, and a dense first layer.

moe_mlp already runs that routing. What the forward did not do is read the pack's way of stating it.

The pieces

what before how it is read now checked against
gating softmax (no expert_gating_func) moe_router_use_sigmoid: true hy-v3.cpp
weight scale 1.0 router_scaling_factor (2.826) hy-v3.cpp
leading dense layer none on the non-MLA path first_k_dense_replace, width intermediate_size; DG/DU/DD designs and dense_mlp, as the MLA path has hy-v3.cpp
expert width moe_intermediate_size = 1536 expert_hidden_dim = 768 tencent/Hy-Memory-30B-A3B: experts and shared expert are [768][2048]
router read flat as [E][H] stored [H][E], transposed at load when that is its shape upstream router.gate.weight is exactly its transpose
expert bias absent (moe_router.bias) moe_expert_bias, F32 (or bf16) equal to upstream mlp.expert_bias
expert slices [K][N] (FLM-name default) [N][K], decided from the first MoE layer's gate stack [E*N/32][K/256][chunk] upstream expert 0, layer 1: gate/down corr 0.9974 as [N][K], 0.008 as [K][N]

The upstream checks are ranged reads of the checkpoint's own tensors. A batched prefill of a non-MLA MoE with a leading dense layer takes the per-token path.

Where else the rule applies. The [N][K] rule needs the stack's per-expert row tiles × 32 to equal the expert width. A transposed Qwen MoE pack has H/32 there (2048 against 768 for Qwen3-Coder-30B). A scan of every *_exps_proj container on this box finds one other pack it fires on, MiniCPM-V-4.7-35B-A3B-NPU2 (#1788's model, found in review). The reading is right there as well: its layer-0 expert 0 against openbmb/MiniCPM-V-4.7-35B-A3B's own gate_up_proj gives corr 0.997 as [N][K] and 0.024 as [K][N], main's reading. That pack stops at MoE designs missing on this forward with the rule on and with it off, so no answer changes; the rule only means its experts will be read correctly once that gap closes. The config-keyed changes touch no other config on the box. The leading-dense path needs first_k_dense_replace or leading_dense_block_count.

Evidence (host, 2026-11-12)

  • Known-answer prompt (the pack's no-think template, greedy): Paris<eos>. Before the slice fix: of of of of of.
  • A numpy hy_v3 forward of the pack's own weights (decoded through q4nx_dequant.c) and this forward, on the same 28-token prompt: logits corr 1.000000, max |d| 3e-5 of 26.6. The top 5 are identical: Paris 26.65, 巴黎 18.79, Paris 17.16, France 15.1, London 12.77.

Tests

New CI arm batch_check_state_hy_v3. It fabricates a two-layer pack written the way this one is:

  • model.layer.N. names, a dense layer 0, and a sigmoid router stored [H][E];
  • an expert bias large enough to change which experts are picked;
  • [N][K] expert stacks declared 3-D, and expert_hidden_dim 512 against a moe_intermediate_size decoy of 1024.

It holds the forward's logits to a numpy hy_v3 on those bytes: corr 1.0000000. Each mutant fails it, while batched == sequential passes every time:

mutant result
slices read [K][N] corr 0.59
no router transpose 0.78
softmax gating 0.9965
no router_scaling_factor 0.94
expert bias ignored 0.87
expert width from moe_intermediate_size refused (expert gate)

The first data set's bias was too small to change a selection, so the bias mutant passed. That also exposed a loader that accepted only an F32 bias; both are fixed.

Suite. Full ctest 270/270, device tests excluded. Every batch_check_state_ci.py arm and gptoss_forward pass. Re-run on the rebased head: build, the 12 cache/MoE/dequant/semantics tests, and the hy_v3 arm. tools/copyright.py --check rc 0.

Not in this PR: a kernel set or device row for this pack.

🤖 Generated with Claude Code

…e way its pack states it (Refs #1786)

hy_v3 routes as DeepSeek-V3 (the fork's hy-v3.cpp), which moe_mlp already runs; the pack said it differently,
and each of these alone made the reply ' of of of':
- config: moe_router_use_sigmoid (was read as softmax), router_scaling_factor, first_k_dense_replace + the dense
  width from intermediate_size, expert_hidden_dim (768; moe_intermediate_size says 1536, upstream experts are
  [768][2048]);
- the router stored [H][E] (upstream's transpose) is transposed at load; moe_expert_bias (F32, or bf16) is read;
- expert slices on an [N][K] grid despite FLM names, decided from the first MoE layer's own gate stack
  (vs upstream expert 0: corr 0.9974 against 0.008 for the old [K][N] reading);
- a non-MLA MoE's leading dense layer gets DG/DU/DD + dense_mlp; a batched prefill takes the per-token path.

Real pack: 'Paris<eos>'; logits vs a numpy hy_v3 forward on its weights corr 1.000000, max |d| 3e-5 of 26.6.
New CI arm batch_check_state_hy_v3 (corr 1.0000000); six mutants fail it. ctest 270/270.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

ready for review at 3763ff6

@github-actions

github-actions Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit fdf4120)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1786 - Partially compliant

Compliant requirements:

  • it imports with zero gaps and its self-check passes against a named reference
  • it serves on --device npu
  • it answers a known-answer prompt with the model's own chat template, 3 identical requests returning identical text
  • the row names the kernel set and the engine commit that produced it

Non-compliant requirements:

  • decode tok/s and time-to-first-token measured under npu-hold

Requires further human verification:

  • decode tok/s and time-to-first-token measured under npu-hold

1788 - Partially compliant

Compliant requirements:

  • imports with zero gaps and passes its self-check against a named reference
  • serves on --device npu
  • answers a known-answer prompt through the model's own chat template, 3 identical requests returning identical text
  • the row names the kernel set and the engine commit that produced it

Non-compliant requirements:

  • a measured decode tok/s and time to first token under npu-hold
  • images: one known image, one known question, an answer that is only obtainable from the picture — not "it accepted the request"
  • tools: a tool-call round trip parsed and answered
  • prefix cache: a repeated prefix (text and a picture) hits the cache and the second TTFT is measurably lower

Requires further human verification:

  • a measured decode tok/s and time to first token under npu-hold
  • images: one known image, one known question, an answer that is only obtainable from the picture — not "it accepted the request"
  • tools: a tool-call round trip parsed and answered
  • prefix cache: a repeated prefix (text and a picture) hits the cache and the second TTFT is measurably lower
⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Incorrect expert slice grid handling

The code attempts to handle the case where expert slices are stored in [N][K] grid format, but the logic for detecting and transposing these slices may not be robust enough. If the detection logic fails, the model may still produce incorrect outputs like " of of of".

{
    char gn[160];
    snprintf(gn, sizeof gn, "model.layers.%d.mlp.gate_exps_proj.weight", leading_dense_ > 0 ? leading_dense_ : 0);
    const TensorDesc* gt = model_tensor_by_name(mw_, gn);
    if (gt && gt->ndim == 3 && n_expert_ > 0 && gt->shape[0] % n_expert_ == 0 &&
        gt->shape[0] / n_expert_ * Q4NX_TILE_ROWS == expert_im_ &&
        gt->shape[1] * Q4NX_TILE_COLS == q4nx_pad_k(c_.H) && expert_im_ != q4nx_pad_k(c_.H)) {
        moe_slice_nk_ = true;
        fprintf(stderr, "  [moe] per-expert slices are [N][K] (%s is [%lld][%lld][...])\n", gn,
                (long long)gt->shape[0], (long long)gt->shape[1]);
    }
}
Inconsistent handling of expert bias

The code attempts to read the expert bias from moe_expert_bias but also falls back to reading from moe_router.bias. This dual approach could lead to inconsistencies if both keys are present or if the fallback logic is not correctly implemented.

if (!load_bf16_named(mn.router_b, moe_router_b_, n_expert_)) {
    moe_router_b_.clear();
    // hy_v3: the expert-selection bias is `moe_expert_bias`, F32 in FastFlowLM's pack (bf16 from our packer)
    const TensorDesc* bd = td_named("moe_expert_bias");
    const void* bp = bd ? model_tensor_data(mw_, const_cast<TensorDesc*>(bd)) : nullptr;
    if (bp && !strcmp(bd->dtype, "F32") && (size_t)bd->data_size >= sizeof(float) * (size_t)n_expert_) {
        const float* fp = (const float*)bp;
        moe_router_b_.assign(fp, fp + n_expert_);
    } else if (bp && !strcmp(bd->dtype, "BF16") && (size_t)bd->data_size >= 2 * (size_t)n_expert_) {
        const uint16_t* hp = (const uint16_t*)bp;
        moe_router_b_.resize((size_t)n_expert_);
        for (int e = 0; e < n_expert_; e++) moe_router_b_[(size_t)e] = bf16_to_f32(hp[e]);
    }
}
Potential memory allocation in hot path

The code includes a section that dynamically allocates memory for transposing the router weights. This allocation happens during model loading and may not be optimal for performance-critical paths. It's important to ensure that such allocations are done once during initialization and not during per-token inference loops.

{
    // A router stored [H][E] (Hy-Memory-30B-A3B's moe_router.weight is upstream's [E][H] transposed,
    // checked against tencent/Hy-Memory-30B-A3B) is read flat as [E][H] by moe_mlp: transpose it.
    const TensorDesc* rd = td_named(mn.router);
    if (rd && rd->ndim == 2 && rd->shape[0] == H && rd->shape[1] == n_expert_ && H != n_expert_) {
        std::vector<float> t((size_t)n_expert_ * H);
        for (int k = 0; k < H; k++)
            for (int e = 0; e < n_expert_; e++) t[(size_t)e * H + k] = moe_router_[(size_t)k * n_expert_ + e];
        moe_router_.swap(t);
    }
}

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at 3763ff6 — the symptom is named before the cause, and the family facts are traced to the reference implementation.

The two symptoms, in the order that makes them useful

"Before, it was refused (moe router), and once loaded its reply was of of of of."

Refusal by name is the diagnostic that makes a pack a day's work; of of of of is the other kind — a model that loads and then emits a degenerate token loop, which reads as a quality problem rather than a routing one. Naming both, in that order, tells a reader that the second was a consequence of the routing fix not being complete, not a separate mystery.

The facts that had to be right

hy_v3 — 128 experts, top-8 — is a shape where a routing bug is invisible in aggregate: with 8 of 128 experts per token, choosing the wrong 8 produces fluent text that is subtly wrong, and a repeated token is the loud end of the same failure. Host-only, which is the only way to verify it under the current NPU-OFF order.

Two asks

  1. say what the router fix was — the body states the symptom and the family facts; the mechanism (how hy_v3's router differs from the MoE path that already worked) is the part a reviewer of the next MoE family needs, and npu-pack: the writer's second key is (layer-elided name, layer type), and it resolves three of four packs #2270/npu-pack: the classifier fixes and the container import refusal, on main (engine#1788) #2277 have set the bar for that sentence;
  2. state the verification that is stronger than "it answers" — for a 128-expert MoE, the cheap strong test is that the same router weights produce the reference's expert choice on a few tokens (correlation or exact ids), and the body says it "now answers on the host". If the answer is the only evidence, say so and mark the routing itself unverified — that distinction is what keeps a family's status honest.

And the scope line is right

"Host-side only; the NPU is off" with Refs #1786 — the same discipline as #2242/#2254/#2261, and the reason this stack of family PRs is reviewable at this rate.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT at 3763ff6: CHANGES

Reviewed at 3763ff67a67ffbddf3808a9c5ed91a9fa8aa6cf8. It is on main (0 behind) and merges clean. The diff is 5 files, +332/-12.

Blocking: the [N][K] slice rule also fires on MiniCPM-V-4.7-35B-A3B-NPU2, so "no other MoE container changes reading" does not hold.

The rule's lookup, model_tensor_by_name("model.layers.%d.mlp.gate_exps_proj.weight"), falls back to the other spelling (model_tensor_name_alt). So it finds MiniCPM-V's model.layer.0.mlp.gate_exps_proj.weight, just as it finds hy_v3's. Its own header and config (~/scratch-packs/MiniCPM-V-4.7-35B-A3B-NPU2) satisfy every clause:

clause MiniCPM-V-35B
gate stack [4096, 8, 4736], ndim == 3
shape[0] % n_expert == 0 4096 % 256 = 0
shape[0]/E * 32 == expert_im_ 16 × 32 = 512 = moe_intermediate_size (no expert_hidden_dim)
shape[1] * 256 == pad_k(H) 8 × 256 = 2048 = H
expert_im_ != pad_k(H) 512 ≠ 2048

So this head sets moe_slice_nk_ for MiniCPM-V and reads its expert slices [N][K], where main read them [K][N] (the FLM-name default). That pack is #1788's model, which has its own lead.

The geometry may well say [N][K] is the right reading for MiniCPM-V too, in which case this also fixes a latent permutation. But that is a claim, and it needs the evidence this PR gave hy_v3:

  • either a ranged read of one MiniCPM-V expert against its upstream checkpoint, as both [N][K] and [K][N] (the corr 0.9974 vs 0.008 test), plus a before/after answer on the pack;
  • or narrow the rule, for example to model_type hy_v3, until MiniCPM-V is measured.

Whichever way it goes, the body's sentence should change.

The same scan found no config on this box that states router_scaling_factor, first_k_dense_replace, expert_hidden_dim, moe_router_use_sigmoid or leading_dense_block_count. The config-keyed changes therefore touch no other pack here; only the shape-keyed slice rule does.

What holds

  • The new batch_check_state_hy_v3 arm reports PASS sequential logits vs the hy_v3 reference on the container's weights: corr 1.0000000 (needs 0.99999), max |d| 1.26e-05 of 8.84, rc 0, with NPU_INFER_HOST_GEMM=1.
  • The other arms (qwen2, lfm3, qwen3_5, default) pass.
  • The mutant table (six mutants, each failing the reference while batched == sequential passes) is the right shape of evidence. I did not re-run it.

CI build at this head was still in progress when I posted.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY -- each piece is justified by an ablation that the test's own threshold rejects, and the row is a real CI gate

The ablation table is the right kind of evidence. Not "we added sigmoid routing", but:

mutant corr
softmax gating 0.9965
no scale 0.94
bias ignored 0.87

against a correct forward at 1.0000000, and LFM3_MIN_CORR = 0.99999 in the harness. So each of those mutants fails by construction, not by assertion order -- the table is enforced by the threshold, and the closest mutant (0.9965) is still 350x further from the bound than the correct value is. That is the shape I want: the pieces are necessary, measured one at a time, rather than a bundle whose necessity nobody can check.

The sentence that explains why this test had to exist is "batched == sequential cannot see it" -- the existing equality check compares the two paths of one implementation, so a wrong reading inside both passes it silently. That is precisely why a numpy reference forward, on the container's own weights, is the only detector here: it is an independent implementation, and correlation against it is the measurement.

And the row is a genuine gate: batch_check_state_hy_v3 fabricates a two-layer pack in a temp dir rather than needing the 30B container, numpy is installed in the build job, and the script's except ImportError: return 77 pairs with the row's SKIP_RETURN_CODE 77 -- so it runs in CI and skips honestly where numpy is absent. That is the difference between this row and the pack-gated ones next door, and it is worth having on a PR whose subject is a 30B container nobody can put in CI.

The reading details are anchored rather than assumed: the router stored [H][E] (upstream's [E][H] transposed, "checked by a ranged read of the checkpoint"), the expert slices on an [N][K] grid where [N][K] gives corr 0.9974 and the old reading gives 0.008, and expert_hidden_dim (512) taking precedence over moe_intermediate_size (1024) because that is what the real pack does -- the same trap #2265 hit from the other direction.

One suggestion, and it is small because the fixture is synthetic: make the three mutants arms rather than a table. The harness already fabricates the pack and already computes correlation, so three more rows (batch_check_state_hy_v3_mutant_softmax, ..._no_scale, ..._bias_ignored) each asserting corr < LFM3_MIN_CORR would turn "each of these mutants fails it" from a claim in the description into something CI proves on every push -- and it would catch the day someone loosens the threshold, which is the only way the table stops being true.

…B, measured right there too (Refs #1786)

Review of #2279 at 3763ff6: the shape rule matches MiniCPM-V-4.7-35B-A3B-NPU2's gate stack ([4096][8][4736],
256 experts of 512). Measured against openbmb/MiniCPM-V-4.7-35B-A3B's own layer-0 gate_up_proj, expert 0:
corr 0.997 as [N][K], 0.024 as [K][N] (main's reading). That pack stops at 'MoE designs missing' on this forward
with the rule on and with it off, so no answer changes. No other container on the box fires the rule.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Answer to the CHANGES verdict at 3763ff6. You were right: the [N][K] rule fires on MiniCPM-V-4.7-35B-A3B-NPU2, so the sentence "no other MoE container changes reading" was false. I took your first option and measured it. Fixed at fdf4120 (docs plus the PR body; no code change).

  • The reading is right for MiniCPM-V too. Its layer-0 expert 0, against openbmb/MiniCPM-V-4.7-35B-A3B's own model.language_model.layers.0.mlp.experts.gate_up_proj (bf16 [256, 1024, 2048], ranged read, gate = rows [:512]), through this PR's q4nx_dequant.c:

    reading gate = rows[:512] gate = rows[::2]
    [N][K] (this head) 0.99716 0.02546
    [K][N] (main) 0.02393 0.00587
  • No answer changes. That pack stops at step failed: MoE designs missing (need D, and G+U or the batched GB/UB) on this forward, with the rule on and with it off. I ran both: the head, and the same build with moe_slice_nk_ forced false. So it changes nothing observable today; once F6: MiniCPM-V 4.7 35B-A3B (MoE vision) on our own NPU runtime — images, tools, prefix cache, measured row #1788 closes that gap, its experts will be read right.

  • Nothing else fires. A scan of every *_exps_proj container on the box (gate_exps_proj shape against config) finds the rule on MiniCPM-V-35B only. Hy-Memory's own pack is deleted after its checks.

  • The PR body now says this in place of the false sentence.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

ready for review at fdf4120

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit fdf4120

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT at fdf4120: READY

Reviewed at fdf41205e5af12ffb230609c746583c82f0a0d7c. It is 1 behind main and merges clean. The new commit fdf41205 touches only docs/npu.md (+5/−1) and the body; the code is unchanged from 3763ff6.

My CHANGES at 3763ff6 is answered. The body no longer says that no other MoE container changes reading. It now names MiniCPM-V-4.7-35B-A3B-NPU2 as the one other pack the [N][K] rule fires on, which is the pack my header scan found, and gives the measurement I asked for:

  • Layer-0 expert 0 against openbmb/MiniCPM-V-4.7-35B-A3B's own gate_up_proj: corr 0.997 as [N][K] and 0.024 as [K][N], main's reading. So the rule is a fix there too: main read MiniCPM-V's experts permuted.
  • No answer changes today. The body says the pack "stops at MoE designs missing on this forward with the rule on and with it off".

Both figures are the author's: the upstream checkpoint is not on this box, and I did not re-run the 35B init. They are stated with the method (a ranged read of the upstream tensor), which is enough to act on.

Carried over from my 3763ff6 review

  • The new arm. batch_check_state_hy_v3 holds the forward to a numpy hy_v3 at corr 1.0000000 (max |d| 1.26e-05 of 8.84). The qwen2, lfm3, qwen3_5 and default arms also pass.
  • The config-keyed changes. No config on this box states router_scaling_factor, first_k_dense_replace, expert_hidden_dim, moe_router_use_sigmoid or leading_dense_block_count, so they touch no other pack.

For #1788's lead: once the MoE-designs gap closes, MiniCPM-V-35B will be read [N][K] on any build with this PR. By the measurement above, that is the correct reading, and any earlier MiniCPM-V MoE numbers came from the permuted reading.

CI build at this head: completed success, "100% tests passed, 0 tests failed out of 272". batch_check_state_hy_v3 is test #254 there and passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant