Skip to content

fix(qwen36): honor RAM_GB as a real ceiling for the expert cache - #1823

Open
Xore wants to merge 1 commit into
JustVugg:devfrom
Xore:fix/qwen36-ram-gb-ceiling
Open

Xore wants to merge 1 commit into
JustVugg:devfrom
Xore:fix/qwen36-ram-gb-ceiling

Conversation

@Xore

@Xore Xore commented Oct 1, 2026

Copy link
Copy Markdown

Splits out the RAM_GB part of #1564, as requested. Rebuilt from scratch on current dev (4521832a) so it carries no unrelated changes.

Problem

RAM_GB is documented as a whole-process memory ceiling, but the expert cache was sized from a request that ignored it. A caller could ask for a cache the machine cannot hold, and the refusal only triggered on a Linux-only /proc/meminfo path, so the same request silently succeeded on macOS and Windows.

Change

  • Extract the clamp into qwen36_cap_for_ram(budget_gb, resident_gb, reserve_gb, slot_gb, layers, requested, n_experts, &left).
  • The clamp may lower a requested cap, never raise one, and never exceed the expert count.
  • It reports the expert budget actually available via the out-param, and reports 0 when not even one slot fits.
  • It reuses the existing cross-platform compat_mem_available_gb() instead of adding a second probe.
  • KV accounting uses the runtime context limit rather than a smaller fixed constant.

c/tests/test_qwen36_ctx.c gains case_ram_budget, covering clamp-down, never-raise, no-slot-fits, and never-exceeds-expert-count.

Verification

Rebuilt onto current dev; no upstream file is removed or reverted (0 deletions of existing lines).

  • make qwen36 — clean, 0 warnings
  • make test-c — full suite passes
  • test_qwen36_ctx — passes
  • Real model, real CUDA, pinned oracle, on the real 35B MoE container:
int8  CUDA_EXPERT_GB=18   Matching tokens: 16/16
int8  CUDA_EXPERT_GB=14   Matching tokens: 16/16
int8  CUDA_EXPERT_GB=10   Matching tokens: 16/16

Identical to pristine dev on the same containers.

Notes

This replaces the RAM_GB portion of #1564. The rest of that PR is still being split out separately; this one stands alone and can be reviewed and merged on its own.

@JustVugg

JustVugg commented Oct 1, 2026

Copy link
Copy Markdown
Owner

Thanks for splitting this out. As it stands we cannot take it, for these reasons:

  1. It adds a feature rather than fixing a bug, and contradicts the docs. docs/qwen36.md says the engine reads no RAM_GB and that --ram changes nothing on qwen36. That may well be worth changing, but then the page has to change with it.
  2. The slot size is wrong in both directions. slot_gb counts one byte per weight. With the recommended int4 gs64 container, the CPU path keeps experts packed (xf_mode, on by default), so a slot is about half that and the clamp halves the cache. With COLI_CUDA=1 a slot holds the int8 copy plus the packed int4 one, about 1.5 bytes per weight, so the same budget can be exceeded.
  3. With RAM_GB set it also rewrites the CUDA tier warm-start (serial expert_get instead of the parallel fill). None of the runs in the description set RAM_GB, so that path has not been exercised.
  4. It adds an undocumented environment variable (COLI_RAM_OVERCOMMIT) and a new exit(2).
  5. It overlaps with feat(qwen36): derive the default expert-cache cap from host RAM instead of a hardcoded 16 #1747, which changes the same cap resolution in model_init_range.

If you want --ram to work on qwen36, the cleanest way is to plan the cap in the launcher, the way coli/the gateway already do for other families, from the container's real slot size, and pass --cap. The engine then stays a consumer of one number. A PR along those lines, with docs/qwen36.md updated, would be welcome.

This branch has not been deployed

No deployments
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.

2 participants