Skip to content

Commit 89d30a3

Browse files
authored
[fix(loader)]: correct off-by-one expert-count guard in load_experts (#2026)
* [fix(loader)]: correct off-by-one expert-count guard in SafeTensorLoader.load_experts After the discovery loop, max_experts_count is the highest expert index found (expert count - 1), and is -1 only when the key has no experts. The guard checked == 0, which falsely rejected single-expert layers and silently returned empty weight lists for the zero-expert case. Check == -1 instead. Adds a CPU regression test covering the single-, zero-, and multi-expert cases. * [test(loader)]: import loader as a top-level module in expert-count guard test Per review feedback: add python/utils to sys.path and import loader directly instead of the importlib.util boilerplate. Still bypasses utils/__init__.py (and the compiled kt_kernel_ext) while keeping the import idiomatic.
1 parent c1cb223 commit 89d30a3

2 files changed

Lines changed: 96 additions & 1 deletion

File tree

kt-kernel/python/utils/loader.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,11 @@ def load_experts(self, base_key: str, device: str = "cpu"):
199199
max_experts_count = -1
200200
while self.has_tensor(f"{up_base_key}.{max_experts_count+1}.numa.{0}.weight"):
201201
max_experts_count += 1
202-
if max_experts_count == 0:
202+
# After the loop max_experts_count is the highest expert index found, i.e.
203+
# (expert count - 1). It stays -1 only when no experts exist for this key.
204+
# (Checking == 0 was an off-by-one: it falsely rejected single-expert layers
205+
# and silently accepted the zero-expert case, returning empty lists.)
206+
if max_experts_count == -1:
203207
raise ValueError(f"No experts found for key {base_key}")
204208
while self.has_tensor(f"{up_base_key}.{0}.numa.{max_numa_id+1}.weight"):
205209
max_numa_id += 1
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
"""Regression tests for SafeTensorLoader.load_experts expert-count guard.
2+
3+
After the discovery loop, ``max_experts_count`` holds the highest expert index
4+
found, i.e. ``(expert count - 1)``. It is ``-1`` only when no experts exist for
5+
the key, so the guard must reject ``-1`` (no experts), not ``0`` (exactly one
6+
expert). The previous ``== 0`` check was an off-by-one that falsely raised
7+
"No experts found" for a single-expert layer and silently returned empty weight
8+
lists for the genuine zero-expert case.
9+
10+
The loader module is imported as a top-level module (its directory is added to
11+
sys.path) so the test does not pull in the compiled kt_kernel_ext extension via
12+
utils/__init__.py.
13+
"""
14+
15+
import os
16+
import sys
17+
import unittest
18+
19+
# Register this test for CPU CI.
20+
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
21+
from ci.ci_register import register_cpu_ci
22+
23+
register_cpu_ci(est_time=5, suite="default")
24+
25+
sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "..", "python", "utils"))
26+
import loader
27+
28+
SafeTensorLoader = loader.SafeTensorLoader
29+
30+
31+
class _FakeTensor:
32+
"""Stand-in for a loaded tensor; load_experts only calls ``.numpy()``."""
33+
34+
def numpy(self):
35+
return 0
36+
37+
38+
class _StubSafeTensorLoader(SafeTensorLoader):
39+
"""SafeTensorLoader backed by an explicit key set instead of real files."""
40+
41+
def __init__(self, keys):
42+
# Bypass the filesystem/safetensors scan in the real __init__.
43+
self.tensor_file_map = {k: "stub.safetensors" for k in keys}
44+
self.file_handle_map = {}
45+
self.tensor_type_map = {}
46+
self.tensor_device_map = {}
47+
48+
def load_tensor(self, key: str, device: str = "cpu"):
49+
assert key in self.tensor_file_map, f"unexpected key requested: {key}"
50+
return _FakeTensor()
51+
52+
53+
def _expert_keys(base_key, n_experts, n_numa=1):
54+
"""Build the NUMA-sharded expert key set that load_experts expects."""
55+
keys = []
56+
for proj in ("ffn_up_exps", "ffn_gate_exps", "ffn_down_exps"):
57+
for expert in range(n_experts):
58+
for numa in range(n_numa):
59+
prefix = f"{base_key}.{proj}.{expert}.numa.{numa}"
60+
keys.append(f"{prefix}.weight")
61+
keys.append(f"{prefix}.scale")
62+
return keys
63+
64+
65+
class TestLoadExpertsCountGuard(unittest.TestCase):
66+
def test_single_expert_is_loaded(self):
67+
"""A layer with exactly one expert must load, not raise."""
68+
loader = _StubSafeTensorLoader(_expert_keys("blk.0", n_experts=1))
69+
result = loader.load_experts("blk.0")
70+
for proj in ("up", "gate", "down"):
71+
self.assertEqual(len(result[proj]), 1, f"{proj}: expected 1 numa group")
72+
self.assertEqual(len(result[proj][0]), 1, f"{proj}: expected 1 expert")
73+
74+
def test_zero_experts_raises(self):
75+
"""No experts under the key must raise, not silently return empties."""
76+
loader = _StubSafeTensorLoader(["blk.0.attn_norm.weight"])
77+
with self.assertRaises(ValueError):
78+
loader.load_experts("blk.0")
79+
80+
def test_multiple_experts_and_numa_counts(self):
81+
"""Counts are correct across several experts and NUMA shards."""
82+
loader = _StubSafeTensorLoader(_expert_keys("blk.0", n_experts=3, n_numa=2))
83+
result = loader.load_experts("blk.0")
84+
for proj in ("up", "gate", "down"):
85+
self.assertEqual(len(result[proj]), 2, f"{proj}: expected 2 numa groups")
86+
for numa_group in result[proj]:
87+
self.assertEqual(len(numa_group), 3, f"{proj}: expected 3 experts")
88+
89+
90+
if __name__ == "__main__":
91+
unittest.main()

0 commit comments

Comments
 (0)