feat(qwen36): derive the default expert-cache cap from host RAM instead of a hardcoded 16 - #1747
jtinbergen wants to merge 4 commits into
Conversation
a7eeaa6 to
9ba50b8
Compare
|
Thanks. With an explicit cap this is fine on the CPU path: the qwen36 oracles are byte-identical to dev, One bug blocks it, on the path this PR is about. When Also a heads-up: the new Makefile rule lists headers by hand and builds several units in one command. Once #1758 lands, its guard rejects rules like that, so it is easier to write it the way #1758 does from the start. |
…ad of a hardcoded 16 cap<=0 (explicit 0, or bare CLI omission) now derives the expert-cache slots/layer from mem_available_gb()*0.88 (or an explicit RAM_GB override), the int4-vs-int8 per-slot byte cost, and the active layer count, instead of always returning 16 regardless of the machine. Mirrors olmoe.c's own cap<=0 budget block and colibri.c's 0.88 margin; qwen36.c had no mem_available_gb() wrapper before this. qwen36_segment_engine_open's memory_limit_bytes==0 path gets the same sentinel treatment, honoring ColiSegmentEngineOptions' documented default-budget promise. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
JustVugg#1757 made num_experts 0 a valid config (Qwen3.8-27B and the other dense checkpoints of the family): the layer MLP loads as an ungated shared expert and nothing is routed, so the per-layer expert cache is never touched by moe()/expert_get(). qwen36_cap_for_ram clamps its derived value against n_experts, which a dense model does not have -- an unclamped RAM budget could size a cache that will sit empty. Short-circuit to cap=1 instead, before the derivation runs. Verified token-exact (16/16) on a qwen38-27b-dense tiny fixture at both an explicit cap and the cap<=0 sentinel; the MoE path is unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Review on JustVugg#1747: main() keeps its own `cap` at the cap<=0 "auto" sentinel. model_init_range resolves it into its own local copy and writes the result to every layer's cache, but main() never reads it back, so the unresolved 0 is what reaches qt_init. qt_init refuses the VRAM expert tier for any cap != n_experts outside fp8-stream mode (qwen36_tier.c:536). Not a regression -- dev's hardcoded 16 is equally != 256 for Qwen3.6-35B-A3B, so a bare invocation never got the tier and callers passed `--cap 256` for it. But reading the value back makes the automatic cap work where the old default could not: with enough RAM the sentinel resolves to n_experts, and the tier comes up without an explicit cap. qwen36_resolved_cap() keeps the shape qwen36_cap_for_ram() established: no Model pointer, no globals, no I/O, so test_qwen36_cap_precedence.c pins it without a container. An explicit cap passes through untouched; a null or empty cache hands the sentinel on rather than inventing a value. The guard sits behind COLI_CUDA and the CPU build links the inline qt_init stub, so no CPU-only test can observe it -- the unit test pins the helper instead, and fails on the pre-fix body. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ustVugg#1758) JustVugg#1758 removed hand-written header lists from 202 rules and renamed QWEN36_TIER_SRC to QWEN36_TIER_OBJ. The rule this PR added predates it and named five headers plus the old variable, so after the rebase onto dev it no longer resolved. Now spelled exactly like its neighbour tests/test_qwen36_slot_int8: one translation unit, $(QWEN36_CFLAGS) so -MMD -MP writes the .d, no header list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
9fe4666 to
617f9de
Compare
|
Thanks. Fixed.
One correction: this was not a regression. Makefile: rebased onto Local:
|
Title:
feat(qwen36): derive the default expert-cache cap from host RAM instead of a hardcoded 16Summary
cap(expert-cache slots/layer,argv[1]) no longer defaults to a hardcoded16when omitted. Bare omission and an explicitcap=0are now the samesentinel ("auto-size from host RAM"), matching the
cap<=0conventioncolibri.c/olmoe.calready use -- this PR only extends that convention tobare CLI omission, which neither of those engines does either today.
qwen36_cap_for_ram(): a pure function (noModel*, no globals, noI/O -- same testability contract as
coli_resolve_cap/k3_cap_for_ram)that derives the cap from
resident + mem_available_gb()*0.88(or anexplicit
RAM_GBoverride), the int4-vs-int8 per-slot byte cost(
xf_mode-aware, half the bytes when experts are stored packed int4), andthe number of active layers. Mirrors
olmoe.c's owncap<=0budget blockand reuses
compat_mem_available_gb()-- the same cross-platform free-RAMprobe every other engine (
colibri.c,olmoe.c,inkling.c,kimi_k3.c,deepseek_v4.c) already wraps in a localmem_available_gb();qwen36.chad no such wrapper before this PR.
RAM_GBas the override name and0.88as the margin are the existing repo-wide convention, not inventedhere. The resolved budget and per-slot size are returned through two
outparams rather than recomputed by the caller, so the diagnostic line
below can never drift from what the function actually decided.
model_init_range, inserted right after all dense weights forthe range finish loading (so
rss_gb()reflects a real, all-dense-residentfootprint) and right before the per-layer expert-cache allocation that
consumes
capfor the first time -- no reordering ofmain()'s argumentparsing needed. Prints
[qwen36] cache auto-sized: N slots/layer of M experts (...)so the derivation is visible and testable, the same wayolmoe.c's own cache-line is.qwen36_segment_engine_open'smemory_limit_bytes == 0path also nowpasses the same sentinel through, instead of leaving
caphardcoded at16--ColiSegmentEngineOptions.memory_limit_bytes's own doc commentalready promised
0means "the adapter's ordinary automatic budget"; thiscloses that gap with the same one function, one line changed. The
memory_limit_bytes != 0path (an explicit caller-supplied byte ceiling,a different concept from measuring host RAM) is unchanged.
cap>0, from either call site, is never touched by any ofthis -- the new code path is only reached when
cap<=0.num_experts == 0, [Feature]: Qwen 3.8 27b #1757's Qwen3.8-27Band the rest of the family) short-circuit to
cap=1before the derivationruns. Nothing is routed there, so the per-layer cache is never touched by
moe()/expert_get(), andqwen36_cap_for_ram'sn_expertsclamp hasnothing to clamp against -- an unclamped RAM budget could otherwise size a
cache that will sit empty. Found while rebasing onto current
dev: thisPR was written when
validate_cfgstill guaranteedn_experts > 0.Known, pre-existing, out-of-scope bug found while reading this code, not
fixed here:
xf_mode()memoizes its int4-vs-int8 answer in a process-globalstatic int, not per-Model*. If a single process opens more than oneqwen36_segment_engine_open()model with different on-disk containerformats, the second model's decode kernel (not just this PR's cap sizing)
would silently reuse the first model's answer. This predates this PR and
lives on the hot decode path (
matmul_d's dispatch,slot_ensure_allocated);fixing it is a separate, higher-risk change and is intentionally not part of
this diff. Flagging it here rather than staying quiet about something found
along the way.
Scope
Default-selection only. No numerical behavior changes anywhere: an explicit
capproduces byte-identical cache allocation/behavior to before this PR(verified directly, see Measured). The auto-derived value itself is a
resource-sizing heuristic, not something with a single correct answer to
verify bit-exactly -- what's verified instead is that it's bounded correctly
(never
<1, never>n_experts), responds to its documented inputs(
RAM_GB, int4 vs int8, layer count), and that a model loads and runscorrectly at whatever cap it derives.
Measured / Demonstrated
Not a throughput change, so
CONTRIBUTING.md's experiment-manifestrequirement (built around
samples.tok_s) doesn't apply here -- samereasoning as #1716's own "Measured" section: this changes what number gets
chosen before the cache exists, not decode speed at a given cap. Demonstrated
instead with real command transcripts on the same real model and machine as
#1716:
qwen36-i4-gs64(35B), Intel Core Ultra 7 155H (16C/22T, hybrid P/E),61 GiB RAM, local ext4 SSD, commit
9fe4666e(rebased ontoorigin/deveefa57a3). Single runs, not medians over several -- thederived cap is a deterministic function of its inputs (resident RSS,
mem_available_gb(), geometry), not a timing measurement, so repeating arun would reproduce the same number rather than add information; the
benchmark-reporting rigor (median + spread over repeated runs) that
CONTRIBUTING.mdasks for applies to noisy throughput claims, which thisPR doesn't make.
Bare omission now auto-sizes instead of defaulting to 16:
Before vs. after, same model, varying available RAM
RAM_GBstands in for "what's actually free" (realmem_available_gb()reads whatever the host has at that moment;
RAM_GBmakes the same codepath reproducible for this table). Same
qwen36-i4-gs64, same machine:The risk on a low-memory system that motivates this: the old default of
16knows nothing about the machine it's running on. On this particularmodel's geometry (~2 MB/slot) that happens not to be dramatic at 6 GB (16
vs. 18) -- but the old number carries no relationship to available RAM at
all, on any model. For a model with a larger
hidden/inter(biggerper-slot cost), the same hardcoded
16could just as easily land abovewhat a tight machine actually has free, with nothing in the old code path
even aware of that -- the cache would still be allowed to grow toward 16
slots/layer regardless of what else is resident, and only a user who already
knew to pass a smaller explicit
capby hand was protected. The new defaultis bounded by the same
mem_available_gb()measurement on every model, soit can't hand out a cache ceiling bigger than what's actually free in the
first place -- on a genuinely tight machine it now picks a smaller cap
than 16 automatically instead of a user finding out the hard way. The
trade-off is the same one already inherent to a smaller cap on any engine:
fewer resident slots means more disk streaming/cache misses during
generation, not incorrect output -- this PR does not change that trade-off,
it only chooses a cap that respects the machine's actual headroom instead of
a number that ignores it.
RAM_GBbounds the choice on the same model (tight budget vs. generous):An explicit cap is never re-derived, regression-checked on the same model:
SNAP=~/models/qwen36-i4-gs64 SERVE=1 ./qwen36 32 4Token-exact oracle, both as a pure regression check and on a cap the new
auto-derivation itself picked (
make_qwen36_tiny.py --ref-mode full+convert_qwen36.py --ebits 8,COLI_DENSE_I8=0, the same tiny fixture andflags the existing
qwen36-tiny-checkCI job uses):Verification
test_qwen36_cap_precedence(new, unit, pure function):RAM_GBoverridewins over the computed budget, and its outparam echoes the override
exactly; a negative or zero override (garbage/unset
RAM_GB) falls backto the computed budget rather than propagating a negative one; int4
derives a cap
>=int8 for identical budget/geometry; a budget at orbelow the resident floor still returns
cap==1, never0; a generousbudget clamps at
n_experts;n_active_layersscales the result(Segment/Edge partial-model ranges pass
layer_end-layer_begin, notalways the full model's
n_layers); degenerate inputs(
n_active_layers<=0) don't crash.test_qwen36_cap_budget.py(new, integration, black-box, mirrorstest_olmoe_cap_budget.py's approach for the sibling engine): explicit capis never second-guessed; a generous
RAM_GBholds every expert; a budgetbelow the floor still runs at
cap==1; the budget bounds the choice; noRAM_GBstill decides (measures the real host); bare CLI omission producesthe identical cache-line as an explicit
cap=0 bits=4-- the equivalencethis PR's whole premise rests on, tested directly rather than only argued
from reading
main().test_segment_adapters_real qwen36 <fixture> 0 4 8 8(existing real-modelSegment/Edge gate, not part of routine
make check-- it needsQWEN_SEGMENT_MODELset to a real converted container, same as everyother engine's entry in the
segment-adapters-realMakefile target): rundirectly against the tiny fixture to exercise
qwen36_segment_engine_open'smemory_limit_bytes==0path end to end,including a genuinely partial layer range (
[0,4)/[4,8)alongside thefull
[0,8)engine in the same process) -- this is the one call site theunit test above can't reach on its own.
qwen36 real Segment range/chaining/snapshot: ok.num_experts == 0): aqwen38-27b-densetiny fixture(
make_qwen36_tiny.py --geometry qwen38-27b-dense, the geometry [Feature]: Qwen 3.8 27b #1757added) is token-exact 16/16 both at an explicit
capand at thecap<=0sentinel, and the sentinel prints no auto-sizing line -- confirming the
short-circuit runs and no oversized cache is allocated.
make check: pass, 1923 tests, 0 new warnings (aftermake clean, on therebased tree).
make test-asan(ASan + UBSan): clean.the new auto-derivation path (see Measured above): 16/16 in all four runs.