Skip to content

feat(win): fix silent CPU fallback, launcher suite, DirectStorage expert loads - #670

Open
khalilswdp wants to merge 4 commits into
JustVugg:devfrom
khalilswdp:feat/windows-gpu-suite
Open

feat(win): fix silent CPU fallback, launcher suite, DirectStorage expert loads#670
khalilswdp wants to merge 4 commits into
JustVugg:devfrom
khalilswdp:feat/windows-gpu-suite

Conversation

@khalilswdp

Copy link
Copy Markdown
Contributor

**Stacked on #667
#667 ** (model-architecture seam) — its commit shows here until
it merges. This PR's own work is three commits, reviewable independently:

  1. fix(win): static cudart + launcher/cap-warning fixes
  2. feat(win): PowerShell launcher suite
  3. feat(win): DirectStorage expert-load transport (opt-in)

Summary

On Windows every run silently fell back to CPU: coli_cuda.dll linked dynamic
cudart, but CUDA 13.x keeps cudart64_*.dll in bin\x64 (not on PATH), so
LoadLibrary failed quietly. Fix: -cudart static (the DLL then imports only
KERNEL32). The rest makes the fix visible and usable:

  • coli launcher: banner prints the real backend instead of a hardcoded
    "streaming CPU"; numeric env vars parse tolerantly (CUDA_EXPERT_GB=auto no
    longer crashes); --ram accepts fractional budgets.
  • cap=1 warning: quotes the actual RAM shortfall and what --ram would
    buy a second cache slot, instead of unconditionally blaming PIN_GB.
  • windows/*.ps1 suite: Build (toolchain detect + doctor), Run (wizard,
    attaches to a running serve instead of starting a second engine), Chat, Web,
    Warmup. Saved with a UTF-8 BOM — PowerShell 5.1 reads BOM-less files as
    ANSI, which breaks the multilingual warmup prompts.
  • warmup.ps1: -Backend auto|gpu|cpu (the .coli_usage pin is
    backend-flavoured), -PromptFile, +40 multilingual/code prompts.
  • DirectStorage expert loads (opt-in COLI_DSTORAGE=1, default off): the
    miss path's coalesced ~19 MB slab pread can be served by a DirectStorage
    host-destination DMA instead. Pure transport swap — bytes land in the slab
    the expert cache already owns; cache/eviction/layout untouched; primary
    replica only; any failure (no SDK at build, no dstorage.dll, no adapter,
    request error) falls back to pread. The depot PR reuses this transport for
    disk→VRAM. No performance claims yet — correctness-verified only.

Validation

  • make -C c check full pass at this tip; zero new warnings
  • make CUDA_DLL=1 colibri.exe clean; DLL builds with and without
    DSTORAGE_HOME
  • DirectStorage A/B on a real int4 container: COLI_DSTORAGE=0 vs 1
    byte-identical output; served loads reported at exit
  • Token-exact oracle with the flag off (default): 32/32 TF, 20/20 greedy
  • All six .ps1 scripts parse under Windows PowerShell 5.1 and PS 7
  • Static-cudart DLL: dumpbin shows KERNEL32 imports only; GPU engages on a
    box where the dynamic link failed silently

Compatibility

  • Default CPU build unchanged and dependency-free — DirectStorage is
    opt-in at build time (DSTORAGE_HOME), optional in the loader;
    Linux/macOS unchanged
  • No model files or binaries included
  • Defaults changed: none in the engine.

@JustVugg

Copy link
Copy Markdown
Owner

Holding this behind #667 (its arch-seam base). Please rebase #667 first onto latest dev, then this one on top — CI green on both, just needs the merges resolved.

@JustVugg JustVugg added the enhancement New feature or request label Jul 28, 2026
@khalilswdp
khalilswdp force-pushed the feat/windows-gpu-suite branch 2 times, most recently from 58a241a to d74d087 Compare July 28, 2026 23:03
@JustVugg

Copy link
Copy Markdown
Owner

dev has moved a fair amount (v1.3.0: Kimi K3 and Inkling engines, the ragged-attention grouped-scale fix, CUDA_RELEASE_HOST default, the Inkling expert-cache fix). This PR now conflicts — could you rebase onto latest dev? Nothing is wrong with the change itself; CI was green where it ran. Ping me once it's clean and I'll review/merge.

@khalilswdp
khalilswdp force-pushed the feat/windows-gpu-suite branch from d74d087 to 490cf3e Compare July 31, 2026 08:07
@JustVugg

JustVugg commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Rebase request — and an apology for how many of these you have had.

dev moved a lot in the last day: 19 PRs landed, including the Vulkan backend (#418), the Kimi K3 GPU tier (#705), Metal grouped-int4 (#457), the shared routing telemetry (#716/#719), and several CUDA and launcher fixes. This PR now conflicts.

Before asking, three things changed on our side so that this is the last one of these you should need for a while.

1. The main source of these conflicts is closed. c/Makefile had a single hand-written TEST_BINS line listing every test gate. Every PR that added a test appended to that same line, so any two such PRs conflicted by construction, even when they touched entirely unrelated code — c/Makefile appeared in 26 of 40 open PRs. #386 hit it twice while being rebased, and said so, which is what sent us looking.

Gates are now derived from the build rules (#733). Adding a test means adding your .c and its own rule, which land in different places in the file. There is no shared list left to conflict on.

2. We resolved what we could ourselves instead of asking. Eight PRs were unblocked by a maintainer pushing the merge to the contributor's branch rather than requesting a rebase — including three where both sides carried a real change and had to be merged rather than picked. No commits were rewritten. Yours is here because its conflict is in engine code (c/colibri.c or a backend), where guessing your intent would be worse than asking.

3. Merge order is oldest-clean-first from now on. A PR that is green and unconflicted merges ahead of anything opened after it. The reason some of you rebased many times is that newer, smaller PRs kept jumping the queue and resetting you — a starvation loop we built, not bad luck on your side.

What we need: one rebase onto current dev. If your only conflict was the TEST_BINS line, just drop your entry — your test is picked up by its own rule now.

If you would rather not, say so and we will close it with thanks and the branch stays yours to reopen. No pressure either way, and no hard feelings — several of these have been open a while through no fault of the author.

@khalilswdp
khalilswdp force-pushed the feat/windows-gpu-suite branch from 490cf3e to 7555dba Compare August 4, 2026 15:31
Same conflict as JustVugg#825 and the same resolution, in c/coli's cmd_run.
dev added the DeepSeek V4 branch (JustVugg#165) right after a plain banner("run");
this branch moved that banner below env_for so it can report the backend --
banner("run", COLI_CUDA=="1").

Kept the V4 branch and gave it its own banner("run"), since that path
sys.exit()s before reaching the CUDA-aware call. Taking either side whole
would have printed two banners or dropped the V4 path.

`python -m ast` parses the result.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ZacharyZcR pushed a commit to ZacharyZcR/colibri that referenced this pull request Aug 5, 2026
`coli chat`, `run`, `info`, `plan` and `tune` all printed

    GLM-5.2 · 744B MoE · int4 · streaming CPU

regardless of what was loaded. Load Inkling, Kimi K3, OLMoE or the new
DeepSeek V4 engine and the third banner line still named GLM.

## Why it was wrong, and where the wrongness lives

Not in the banner. In what the banner would have had to ask:

    def model_arch(model):
        ...
        return "glm"          # <- every unrecognised model_type

That default is correct for its job -- colibri.c is the general engine, so
an unknown checkpoint should be offered to it. It is the wrong answer to a
different question, "what did the user load", and the banner needs that
one. So `model_banner_line()` reads the raw `model_type` instead, and the
dispatch in `model_arch()` is untouched.

## What it prints

    GLM-5.2 · 744B MoE · 372 GB on disk
    DeepSeek V4 Flash · 284B MoE · 167 GB on disk
    OLMoE · 7B MoE · 4.2 GB on disk
    qwen3_moe · 48L x 128E MoE · 61 GB on disk      <- not in the roster

Parameter counts come from the README roster, so the two cannot drift apart
without someone noticing. A model_type that is not in the table is NOT
forced into a name: it prints its own type and the geometry measured from
its config, which is honest and still useful. No config.json at all, or an
unreadable path, keeps the original tagline -- `coli info` banners before it
validates the model directory, so this has to be safe on a bad path.

Size is measured by stat-ing the shards; no safetensors headers are parsed.
A banner runs before every command and may not cost a scan of a 400 GB
checkpoint.

## Note for the open PRs that also touch banner()

JustVugg#825 and JustVugg#670 both change `banner("run")` to pass a CUDA flag positionally.
`model=` is therefore **keyword-only**, so a second positional argument
raises TypeError instead of being silently read as a path. There is a test
for exactly that.

Eight tests in tests/test_cli_output.py, including the regression itself: a
`deepseek_v4` config must not produce a line containing "GLM" or "744B".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request needs-rebase Confligge, serve rebase dell'autore

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants