Skip to content

Commit a08ffd1

Browse files
solderzzcclaude
andauthored
test: make the MoE fixture actually test nesting, and fix two harness faults (#145)
Review follow-ups on #143/#144. All three findings were in code I wrote. **The MoE fixture tested nothing it claimed.** It used `qwen3_moe`, whose Swift configuration decodes from the root of config.json, so the expert count had to be at the top level; the nested copy underneath was never reached, because findExpertCounts is breadth-first and returns on the first hit. Both advertised properties — the nesting, and the `vision_config` decoy that "must not be mistaken" — were dead weight. Deleting the nested walk entirely would not have failed it. Rebuilt on `gemma4`, whose Gemma4Configuration decodes `text_config` and nothing else, so the count exists only one level down and the decoy is genuinely reached. `gemma4` also contains no "moe", so modelTypeImpliesMoE cannot rescue it — which is what made the qwen3_moe version untestable. Red-green verified, the check the previous version could not pass: reverting detection to the pre-#114 top-level single-key form fails the new assertion, and restoring it passes. The fixture now reproduces #112. The assertion also moved to the right gate. There are two: a config-level MoE check, and a model-level StreamableMoE conformance check. gemma4 passes the first and legitimately declines the second, so asserting "streaming enabled" would have tested the wrong thing. It now asserts only that detection did not reject. Incidentally covers the fused-expert remap — real gemma4 checkpoints ship `experts.gate_up_proj` as one tensor that sanitize splits into `switch_glu.gate_proj`/`up_proj`. A wrong split axis is a silent numerical fault. **Two harness faults.** cleanup() killed the server without waiting, and the readiness loop probed health before checking liveness. If a teardown outlived the 1s sleep, the next fixture would fail to bind, and its first probe would be answered by the previous server — assertions then run against the wrong checkpoint and report a false pass, not a flake. cleanup now waits, and liveness is checked first. The generator wrote into existing directories without clearing them, so files a builder stopped emitting survived and the fixture kept testing a shape the source no longer described. Each fixture's own directory is now cleared first — scoped to one known directory, which is also what keeps regeneration away from siblings like tests/fixtures/omni, whose assets belong to test-omni.sh. Suite: 6 passed, 0 failed, twice consecutively. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 664a05d commit a08ffd1

4 files changed

Lines changed: 139 additions & 84 deletions

File tree

‎scripts/make-test-fixtures.py‎

Lines changed: 82 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
"""
2424
import json
2525
import os
26+
import shutil
2627
import sys
2728

2829
import numpy as np
@@ -230,77 +231,107 @@ def build_gemma4_kv_shared(out, vestigial):
230231

231232

232233
def build_moe_nested(out):
233-
"""A Qwen3-style MoE: per-expert tensors, nested config, decoy vision count.
234-
235-
Covers the MoE *load* path, which nothing else in CI touches — no MoE model is in
236-
the matrix at all. Specifically: per-expert `mlp.experts.N.*` tensors being stacked
237-
into `switch_mlp` by sanitize, and an expert count nested under `text_config`
238-
rather than at top level.
239-
240-
What it does NOT cover, despite the nesting, is the #112 detection bug itself.
241-
That bug was `num_local_experts` being the only spelling checked, so a config
242-
declaring `num_experts` was called dense. But `modelTypeImpliesMoE` treats any
243-
model_type containing "moe" as MoE regardless of keys, so `qwen3_moe` is caught by
244-
that fallback whatever the config says — verified by reverting detection to the
245-
single-key form and watching this fixture still enable streaming.
246-
247-
Reproducing #112 end to end needs a MoE architecture whose model_type does not
248-
contain "moe" — deepseek_v3 (`n_routed_experts`) is the candidate. Its MLA
249-
attention makes that a larger fixture; the profiler itself is covered directly by
250-
ModelProfilerMoEDetectionTests in the meantime.
234+
"""Gemma 4 with a MoE text block, so the expert count is genuinely nested.
235+
236+
An earlier version of this fixture used `qwen3_moe`, whose Swift configuration is
237+
decoded from the root of config.json — so the expert count had to sit at the top
238+
level, and the "nested" copy underneath it was never reached. `findExpertCounts`
239+
is breadth-first and returned on the root hit, which made both the nesting and the
240+
vision decoy dead weight.
241+
242+
`Gemma4Configuration` decodes `text_config` and nothing else, so here the count
243+
exists *only* one level down. That makes two things real rather than decorative:
244+
the nested walk added in #112's review follow-up, and the rule that a count under
245+
`vision_config` must not be mistaken for the language model's.
246+
247+
It also covers the fused-expert remap: real gemma4 checkpoints ship
248+
`experts.gate_up_proj` as one tensor that sanitize splits in half into
249+
`switch_glu.gate_proj` / `switch_glu.up_proj`. A wrong split axis, or swapped
250+
halves, is a silent numerical fault no unit test here would see.
251251
"""
252252
H, L, HEADS, KVH, HD = 64, 2, 4, 2, 16
253253
INTER, MOE_INTER, EXPERTS, TOPK = 128, 32, 4, 2
254+
PLI, VPLI = 32, 16
254255
rng = np.random.default_rng(0)
255256

256257
text_config = {
257-
"model_type": "qwen3_moe",
258+
"model_type": "gemma4_text",
258259
"hidden_size": H,
259260
"num_hidden_layers": L,
260261
"intermediate_size": INTER,
261262
"num_attention_heads": HEADS,
262263
"num_key_value_heads": KVH,
263264
"head_dim": HD,
264-
"num_experts": EXPERTS,
265-
"num_experts_per_tok": TOPK,
266-
"moe_intermediate_size": MOE_INTER,
267-
"decoder_sparse_step": 1,
268-
"mlp_only_layers": [],
265+
"global_head_dim": HD,
269266
"rms_norm_eps": 1e-6,
270267
"vocab_size": VOCAB,
268+
"rope_traditional": False,
271269
"rope_theta": 10000.0,
272-
"tie_word_embeddings": False,
270+
"sliding_window": 128,
271+
"sliding_window_pattern": 1,
273272
"max_position_embeddings": 512,
274-
"norm_topk_prob": True,
273+
"num_kv_shared_layers": 0,
274+
"use_double_wide_mlp": False,
275+
"tie_word_embeddings": True,
276+
"hidden_size_per_layer_input": PLI,
277+
"vocab_size_per_layer_input": VPLI,
278+
"final_logit_softcapping": 30.0,
279+
"attention_k_eq_v": False,
280+
"enable_moe_block": True,
281+
"num_experts": EXPERTS,
282+
"top_k_experts": TOPK,
283+
"moe_intermediate_size": MOE_INTER,
284+
}
285+
# num_experts appears here and nowhere else at the root — that is the point.
286+
cfg = {
287+
"model_type": "gemma4",
288+
"architectures": ["Gemma4ForConditionalGeneration"],
289+
"vocab_size": VOCAB,
290+
"text_config": text_config,
291+
# A decoy the language-model walk has to skip.
292+
"vision_config": {"model_type": "gemma4_vision", "num_experts": 999},
275293
}
276-
cfg = dict(text_config)
277-
cfg["text_config"] = text_config
278-
# Must not be mistaken for the language model's expert count.
279-
cfg["vision_config"] = {"model_type": "qwen3_vl", "num_experts": 999}
280294
json.dump(cfg, open(os.path.join(out, "config.json"), "w"), indent=2)
281295

282296
w = {
283-
"model.embed_tokens.weight": rand(rng, VOCAB, H),
284-
"model.norm.weight": ones(H),
285-
"lm_head.weight": rand(rng, VOCAB, H),
297+
"language_model.model.embed_tokens.weight": rand(rng, VOCAB, H),
298+
"language_model.model.norm.weight": ones(H),
299+
"language_model.model.embed_tokens_per_layer.weight": rand(rng, VPLI, L * PLI),
300+
"language_model.model.per_layer_model_projection.weight": rand(rng, L * PLI, H),
301+
"language_model.model.per_layer_projection_norm.weight": ones(PLI),
286302
}
287303
for i in range(L):
288-
p_ = f"model.layers.{i}"
304+
# A gemma4 wrapper checkpoint prefixes its text weights this way; matches
305+
# what a real gemma-4-e2b ships.
306+
p_ = f"language_model.model.layers.{i}"
289307
w[f"{p_}.self_attn.q_proj.weight"] = rand(rng, HEADS * HD, H)
290308
w[f"{p_}.self_attn.k_proj.weight"] = rand(rng, KVH * HD, H)
291309
w[f"{p_}.self_attn.v_proj.weight"] = rand(rng, KVH * HD, H)
292310
w[f"{p_}.self_attn.o_proj.weight"] = rand(rng, H, HEADS * HD)
293311
w[f"{p_}.self_attn.q_norm.weight"] = ones(HD)
294312
w[f"{p_}.self_attn.k_norm.weight"] = ones(HD)
313+
w[f"{p_}.layer_scalar"] = np.ones(1, np.float16)
314+
w[f"{p_}.mlp.gate_proj.weight"] = rand(rng, INTER, H)
315+
w[f"{p_}.mlp.up_proj.weight"] = rand(rng, INTER, H)
316+
w[f"{p_}.mlp.down_proj.weight"] = rand(rng, H, INTER)
295317
w[f"{p_}.input_layernorm.weight"] = ones(H)
296318
w[f"{p_}.post_attention_layernorm.weight"] = ones(H)
297-
w[f"{p_}.mlp.gate.weight"] = rand(rng, EXPERTS, H)
298-
# Per-expert tensors, the layout a real checkpoint ships; sanitize stacks
299-
# these into switch_mlp.
300-
for e in range(EXPERTS):
301-
w[f"{p_}.mlp.experts.{e}.gate_proj.weight"] = rand(rng, MOE_INTER, H)
302-
w[f"{p_}.mlp.experts.{e}.up_proj.weight"] = rand(rng, MOE_INTER, H)
303-
w[f"{p_}.mlp.experts.{e}.down_proj.weight"] = rand(rng, H, MOE_INTER)
319+
w[f"{p_}.pre_feedforward_layernorm.weight"] = ones(H)
320+
w[f"{p_}.post_feedforward_layernorm.weight"] = ones(H)
321+
w[f"{p_}.per_layer_input_gate.weight"] = rand(rng, PLI, H)
322+
w[f"{p_}.per_layer_projection.weight"] = rand(rng, H, PLI)
323+
w[f"{p_}.post_per_layer_input_norm.weight"] = ones(H)
324+
# Router and experts are siblings of mlp on the decoder layer, not nested in it.
325+
# Enabling the MoE block also builds a second pair of feedforward norms.
326+
w[f"{p_}.pre_feedforward_layernorm_2.weight"] = ones(H)
327+
w[f"{p_}.post_feedforward_layernorm_1.weight"] = ones(H)
328+
w[f"{p_}.post_feedforward_layernorm_2.weight"] = ones(H)
329+
w[f"{p_}.router.proj.weight"] = rand(rng, EXPERTS, H)
330+
w[f"{p_}.router.scale"] = ones(H)
331+
w[f"{p_}.router.per_expert_scale"] = ones(EXPERTS)
332+
# Fused, as a real checkpoint ships it: sanitize splits dim -2 in half.
333+
w[f"{p_}.experts.gate_up_proj"] = rand(rng, EXPERTS, 2 * MOE_INTER, H)
334+
w[f"{p_}.experts.down_proj"] = rand(rng, EXPERTS, H, MOE_INTER)
304335

305336
save_file(w, os.path.join(out, "model.safetensors"), metadata={"format": "pt"})
306337
return len(w)
@@ -319,7 +350,16 @@ def build_moe_nested(out):
319350
total = 0
320351
for name, (fn, kwargs) in FIXTURES.items():
321352
out = os.path.join(ROOT, name)
322-
os.makedirs(out, exist_ok=True)
353+
# Clear this fixture's own directory, and only its own. Writing over an existing
354+
# one leaves behind files the current builder no longer emits — flip stray-shard
355+
# off and its decoy shard and index survive, so the fixture keeps testing a shape
356+
# the source no longer describes, and the printed size counts files that are not
357+
# part of it. Scoping the removal to one known fixture directory is also what
358+
# keeps regeneration from reaching siblings such as tests/fixtures/omni, whose
359+
# assets belong to test-omni.sh and are not generated here.
360+
if os.path.isdir(out):
361+
shutil.rmtree(out)
362+
os.makedirs(out)
323363
n = fn(out, **kwargs)
324364
write_tokenizer(out)
325365
size = sum(os.path.getsize(os.path.join(out, f)) for f in os.listdir(out))
Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,39 @@
11
{
2-
"model_type": "qwen3_moe",
3-
"hidden_size": 64,
4-
"num_hidden_layers": 2,
5-
"intermediate_size": 128,
6-
"num_attention_heads": 4,
7-
"num_key_value_heads": 2,
8-
"head_dim": 16,
9-
"num_experts": 4,
10-
"num_experts_per_tok": 2,
11-
"moe_intermediate_size": 32,
12-
"decoder_sparse_step": 1,
13-
"mlp_only_layers": [],
14-
"rms_norm_eps": 1e-06,
2+
"model_type": "gemma4",
3+
"architectures": [
4+
"Gemma4ForConditionalGeneration"
5+
],
156
"vocab_size": 288,
16-
"rope_theta": 10000.0,
17-
"tie_word_embeddings": false,
18-
"max_position_embeddings": 512,
19-
"norm_topk_prob": true,
207
"text_config": {
21-
"model_type": "qwen3_moe",
8+
"model_type": "gemma4_text",
229
"hidden_size": 64,
2310
"num_hidden_layers": 2,
2411
"intermediate_size": 128,
2512
"num_attention_heads": 4,
2613
"num_key_value_heads": 2,
2714
"head_dim": 16,
28-
"num_experts": 4,
29-
"num_experts_per_tok": 2,
30-
"moe_intermediate_size": 32,
31-
"decoder_sparse_step": 1,
32-
"mlp_only_layers": [],
15+
"global_head_dim": 16,
3316
"rms_norm_eps": 1e-06,
3417
"vocab_size": 288,
18+
"rope_traditional": false,
3519
"rope_theta": 10000.0,
36-
"tie_word_embeddings": false,
20+
"sliding_window": 128,
21+
"sliding_window_pattern": 1,
3722
"max_position_embeddings": 512,
38-
"norm_topk_prob": true
23+
"num_kv_shared_layers": 0,
24+
"use_double_wide_mlp": false,
25+
"tie_word_embeddings": true,
26+
"hidden_size_per_layer_input": 32,
27+
"vocab_size_per_layer_input": 16,
28+
"final_logit_softcapping": 30.0,
29+
"attention_k_eq_v": false,
30+
"enable_moe_block": true,
31+
"num_experts": 4,
32+
"top_k_experts": 2,
33+
"moe_intermediate_size": 32
3934
},
4035
"vision_config": {
41-
"model_type": "qwen3_vl",
36+
"model_type": "gemma4_vision",
4237
"num_experts": 999
4338
}
4439
}
89.5 KB
Binary file not shown.

‎tests/test-fixtures.sh‎

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@
1010
# stray-shard #118: a .safetensors beside the index but absent from it
1111
# kv-shared-absent #120: gemma-4-e4b shape, shared layers ship no k/v
1212
# kv-shared-present b674: gemma-4-e2b shape, shared layers ship k/v anyway
13-
# moe-nested MoE load path: per-expert tensors stacked into switch_mlp,
14-
# expert count nested under text_config. Note this does not
15-
# reproduce #112 — see scripts/make-test-fixtures.py for why.
13+
# moe-nested #112: expert count nested under text_config only, with a
14+
# decoy count under vision_config; plus the fused
15+
# experts.gate_up_proj split that sanitize performs
1616
#
1717
# The output is gibberish by construction — the weights are random. A fixture passes
1818
# when the server loads it and produces *a* token, which is what exercises config
@@ -35,7 +35,16 @@ pass() { PASS=$((PASS + 1)); echo -e " ${GREEN}✅ PASS${NC}: $*"; }
3535
fail() { FAIL=$((FAIL + 1)); echo -e " ${RED}❌ FAIL${NC}: $*"; }
3636

3737
SERVER_PID=""
38-
cleanup() { [ -n "$SERVER_PID" ] && kill "$SERVER_PID" 2>/dev/null; SERVER_PID=""; }
38+
# `kill` only asks. Without waiting for the process to go, the next fixture binds the
39+
# same port while the previous server may still hold it — see the readiness loop below
40+
# for why that is worse than a flake.
41+
cleanup() {
42+
if [ -n "$SERVER_PID" ]; then
43+
kill "$SERVER_PID" 2>/dev/null
44+
wait "$SERVER_PID" 2>/dev/null
45+
fi
46+
SERVER_PID=""
47+
}
3948
trap cleanup EXIT
4049

4150
run_fixture() {
@@ -52,10 +61,14 @@ run_fixture() {
5261
"$BINARY" --model "$dir" --port "$PORT" --host "$HOST" > "$logfile" 2>&1 &
5362
SERVER_PID=$!
5463

64+
# Liveness is checked *before* the health probe on purpose. The other order lets a
65+
# server that failed to bind (because the previous one still held the port) pass as
66+
# ready on a probe answered by that previous server — the assertions then run
67+
# against the wrong checkpoint and report a false pass rather than a failure.
5568
local ready=0
5669
for _ in $(seq 1 60); do
57-
if curl -sf "$url/health" >/dev/null 2>&1; then ready=1; break; fi
5870
if ! kill -0 "$SERVER_PID" 2>/dev/null; then break; fi
71+
if curl -sf "$url/health" >/dev/null 2>&1; then ready=1; break; fi
5972
sleep 1
6073
done
6174

@@ -97,29 +110,36 @@ for name in dense stray-shard kv-shared-absent kv-shared-present moe-nested; do
97110
run_fixture "$name"
98111
done
99112

100-
# The MoE fixture again, this time with --stream-experts. A model classified dense
101-
# has the flag silently dropped and gets materialised whole, which is how #112
102-
# reached an OOM kill rather than an error message; asserting the flag was honoured
103-
# is the closest end-to-end check available.
104-
log "Shape: moe-nested (--stream-experts honoured)"
113+
# The MoE fixture again with --stream-experts, to assert the *config-level* gate.
114+
#
115+
# #112: expert counts are spelled and nested differently per family, and a model
116+
# misread as dense had --stream-experts silently dropped and was then materialised
117+
# whole until the OS killed it. The check is that the config gate does not reject —
118+
# not that streaming actually engages. Those are two separate gates: gemma4 passes
119+
# detection but has no StreamableMoE conformance, so the second one legitimately
120+
# declines. Asserting on the first is what tracks #112.
121+
#
122+
# This is a live check rather than a decorative one: `gemma4` contains no "moe", so
123+
# the model_type fallback in modelTypeImpliesMoE cannot rescue it, and the count
124+
# exists only inside text_config. Reverting detection to a top-level single-key form
125+
# makes this fail.
126+
log "Shape: moe-nested (nested expert count is detected)"
105127
MOE_LOG="/tmp/SwiftLM-test-fixture-moe-stream.log"
106128
"$BINARY" --model "$FIXTURE_DIR/moe-nested" --port "$PORT" --host "$HOST" \
107129
--stream-experts > "$MOE_LOG" 2>&1 &
108130
SERVER_PID=$!
109131
ready=0
110132
for _ in $(seq 1 60); do
111-
curl -sf "http://$HOST:$PORT/health" >/dev/null 2>&1 && { ready=1; break; }
112133
kill -0 "$SERVER_PID" 2>/dev/null || break
134+
curl -sf "http://$HOST:$PORT/health" >/dev/null 2>&1 && { ready=1; break; }
113135
sleep 1
114136
done
115137
if [ "$ready" -ne 1 ]; then
116138
fail "moe-nested did not start with --stream-experts"
117-
elif grep -qi "is not MoE" "$MOE_LOG"; then
118-
fail "moe-nested was classified dense: $(grep -i 'is not MoE' "$MOE_LOG" | head -1)"
119-
elif grep -q "SSD Expert Streaming enabled" "$MOE_LOG"; then
120-
pass "moe-nested detected as MoE, --stream-experts honoured"
139+
elif grep -q "is not MoE" "$MOE_LOG"; then
140+
fail "nested expert count missed: $(grep 'is not MoE' "$MOE_LOG" | head -1)"
121141
else
122-
fail "moe-nested: no streaming confirmation in log"
142+
pass "moe-nested: expert count found under text_config, vision decoy ignored"
123143
fi
124144
cleanup
125145

0 commit comments

Comments
 (0)