Skip to content

Fix fast-model verification and align single-mode model selection - #126

Merged
mhenrichsen merged 2 commits into
syv-ai:mainfrom
TyroneNel:fix/f1-fast-model-verifier
Sep 23, 2026
Merged

mhenrichsen merged 2 commits into
syv-ai:mainfrom
TyroneNel:fix/f1-fast-model-verifier

Conversation

@TyroneNel

@TyroneNel TyroneNel commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Closes #125

Scope after review

Rebased onto upstream main at d1df6dc (the README/docs restructure), which includes #122. This PR no longer changes verify.sh: upstream's packed-geometry and shard-header validation is retained unchanged.

Changes

  • Share single-user model selection between the launcher and Docker entrypoint, resolving after preparation and exporting MODEL before verification.

  • Preserve explicit MODEL overrides, base fallback, batch defaults, and standalone verifier defaults.

  • Keep the GPU-free model-verification CI job, entrypoint selection fixture, and Docker documentation.

  • Replace empty safetensors fixtures with header-only shards: 8-byte little-endian header length, padded JSON header, tensor shapes, dtypes, and offsets. No tensor payload is required by these header-only checks.

  • Cover valid int4/int8 geometry, declared int8 over int4 tensors and the reverse, unsupported width, missing group, missing packed head, and missing scale. Rejections must be verifier failures, not Python crashes.

Validation

  • wsl python3 /mnt/g/dev/qwen38-27b-rtx3090/bench/test_model_verification.py: PASS (2 test methods; 8 head cases and 4 selection cases).

  • Exact current upstream model-check heredoc executed with Python against both installed base and fast model directories: exit 0 for both, including packed geometry and duplicate-shard checks.

  • bash -n passed for verify.sh, docker/entrypoint.sh, single-user/start_qwen.sh, and single-user/select_model.sh.

  • Selection fixture executes the actual entrypoint in a temporary root with verifier/launcher path recorders. Covers base fallback, fast auto-selection, explicit path containing a space, and batch default.

Limits

No full verify.sh run, image rebuild, or GPU server restart. The prior container was already stopped, so installed-model headers were checked directly through its host bind mount without modifying model files. Run the fixture suite on Linux/WSL (as CI does); native Windows Python launching WSL bash does not reliably forward its environment.

Rebased onto current main (2026-09-22)

The branch was conflicting after #138 (resolve_config.sh) landed a few lines from the MODEL block, and after #134 added a docs/docker.md bullet next to the one this PR extends. Rebased onto main at 8b4dccd; both conflicts were adjacent additions, not competing edits:

No content changed in the rebase. Re-run on the rebased tip, not carried over:

  • bench/test_model_verification.py — 2 tests, 12 cases, pass (WSL, Python 3.12.3).
  • git-apply and model-verification both green; the PR is 6 files, MERGEABLE / CLEAN.

@mhenrichsen

Copy link
Copy Markdown
Contributor

Half of this is landing as-is and half is superseded — details so you can rebase without guessing.

The verify.sh hunk: #122 went in instead. Same false rejection, stricter fix. Both accept 4 and 8; #122 additionally checks the packed geometry the declared width implies against the shard header. On a fast dir whose config was edited to claim num_bits: 8 over unchanged int4 tensors, run on the real model dirs on the 3090:

A verifier that reads the declared width and never looks at what is on disk has given up the thing it is for, so I took the one that looks.

This breaks your head fixture, concretely. test_supported_and_invalid_heads builds its model dir with (model / "weights.safetensors").touch(). Against current main it now fails both passing cells:

FAIL (bits=8, packed=True): struct.error: unpack requires a buffer of 8 bytes
FAIL (bits=4, packed=True): struct.error: unpack requires a buffer of 8 bytes

An empty file has no safetensors header to read. The fixture needs a real one — an 8-byte little-endian length followed by a JSON header naming lm_head.weight_packed and lm_head.weight_scale with the shapes the declared width implies is enough; no tensor data required, the check is header-only. That also lets the fixture cover the case that separates the two fixes, which is worth having in CI.

The rest is good and I want it. single-user/select_model.sh shared between the launcher and the Docker gate is the right fix for the half of #125 that #122 does not touch — verifying one model and serving another is a genuine footgun, and resolving after preparation is the correct order. The selection fixture executing the real entrypoint with recorder stubs is a nicer test than I expected for something that used to need a container. docs/docker.md stating the contract earns its lines.

So: drop the verify.sh hunk, rebuild the head fixture on real headers, keep everything else. The model-verification CI job is worth adding on its own — a GPU-free regression gate for this is exactly what the repo lacks, and today it would have caught the thing it is about to be broken by.

Leaving #125 open until this lands, since the selection half of it is still live.

@TyroneNel

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed review and the concrete geometry example — agreed that #122 is the stronger verifier fix.

I've rebased onto current main (2cf1002) and dropped our verify.sh hunk entirely. The shared model-selection helper, post-preparation Docker selection, entrypoint fixture, documentation, and GPU-free CI job remain.

The head fixture now writes real safetensors headers rather than empty files. It covers matching int4/int8 geometry, both declared/stored-width mismatch directions, and missing/unsupported heads and scales. It also checks that rejection cases are reported failures rather than crashes.

Validation: all 8 head cases and 4 selection cases pass under Linux/WSL. The unchanged upstream model-check block also passes against both installed model directories, including geometry and duplicate-shard checks. No model files were changed and no server was restarted; this was not a full install-verifier run.

Pushed the rebased branch through f4a9d0c and updated the PR description with the evidence and limits. Thanks again for keeping the selection half of #125 in view!

TyroneNel pushed a commit to TyroneNel/qwen38-27b-rtx3090 that referenced this pull request Sep 17, 2026
# Conflicts:
#	single-user/start_qwen.sh
TyroneNel pushed a commit to TyroneNel/qwen38-27b-rtx3090 that referenced this pull request Sep 17, 2026
# Conflicts:
#	.github/workflows/docker-image.yml
#	bench/test_model_verification.py
#	docker-compose.yml
#	docker/prepare.sh
#	patches/dflash2-ngram-chains.patch
#	patches/dflash2-prewarm.patch
#	patches/dflash2-z-adaptive-emitted.patch
#	patches/hybrid-sw-block-promote.patch
#	patches/vllm-pr54282-draft-gumbel-salt.patch
#	prepare/quant_heads_stream.py
#	single-user/start_qwen.sh
#	verify.sh
@TyroneNel
TyroneNel force-pushed the fix/f1-fast-model-verifier branch from f4a9d0c to 2203c00 Compare September 17, 2026 12:32
@TyroneNel

Copy link
Copy Markdown
Contributor Author

Rebased onto d1df6dc — the branch was 5 commits behind after #129–#133 (the README/docs/ restructure) landed, so the base named in the description was stale.

git merge-tree reported the merge back into upstream/main clean before I started, and the rebase itself applied without a conflict. Verification re-run on the rebased tip, not carried over:

  • bench/test_model_verification.py — PASS (2 test methods; 8 head cases, 4 selection cases) on Linux/WSL.
  • bash -n — clean for verify.sh, docker/entrypoint.sh, single-user/start_qwen.sh, single-user/select_model.sh.

Head is now 2203c00; the description's base reference is updated to match. No content changed in the rebase.

@mhenrichsen

Copy link
Copy Markdown
Contributor

Merging. Everything I asked for is here, and I re-ran the evidence rather than reading it.

On the box (CPU only — the 3090 is on a training job, which none of this needs):

The mismatch coverage in both directions is the part worth having in CI. That case — a config declaring one width over the other's tensors — is the one that separates a verifier that looks at the disk from one that reads the config and shrugs, and until now nothing tested it.

single-user/select_model.sh closes the half of #125 that #122 could not: verifying one directory and serving another is the kind of bug that wastes a day, and resolving after preparation is the right order because prepare is what creates the fast variant.

Two follow-ups, neither blocking:

Thanks for the rebase through the docs/ restructure — that landed in the middle of your PR and was my doing.

TyroneNel added a commit to TyroneNel/qwen38-27b-rtx3090 that referenced this pull request Sep 21, 2026
# Conflicts:
#	single-user/start_qwen.sh
TyroneNel added a commit to TyroneNel/qwen38-27b-rtx3090 that referenced this pull request Sep 21, 2026
# Conflicts:
#	.github/workflows/docker-image.yml
#	bench/test_model_verification.py
#	docker-compose.yml
#	docker/prepare.sh
#	patches/dflash2-ngram-chains.patch
#	patches/dflash2-prewarm.patch
#	patches/dflash2-z-adaptive-emitted.patch
#	patches/hybrid-sw-block-promote.patch
#	patches/vllm-pr54282-draft-gumbel-salt.patch
#	prepare/quant_heads_stream.py
#	single-user/start_qwen.sh
#	verify.sh
Ronel added 2 commits September 22, 2026 21:02
Accept supported packed int4 and int8 heads and select the same model before Docker verification and launch. Preserve explicit overrides and batch defaults. Add CPU regression checks for head formats and selection.

Fixes syv-ai#125
Use header-only shards, reject declared/stored bit-width mismatches in both directions, and retain model-selection regression coverage. Keep upstream syv-ai#122 verifier unchanged.
@TyroneNel
TyroneNel force-pushed the fix/f1-fast-model-verifier branch from 2203c00 to 5fad43d Compare September 22, 2026 19:02
TyroneNel pushed a commit to TyroneNel/qwen38-27b-rtx3090 that referenced this pull request Sep 22, 2026
…#113)

bench/warmup.sh re-derived the model dir (fast variant when present, else the base dir) with a comment saying it mirrors single-user/start_qwen.sh. syv-ai#126 extracts exactly that resolution into single-user/select_model.sh and points the launcher and the Docker gate at it, which leaves this copy as the only one left to drift. Source the same file here.

Equivalence, not approximation: the sourced resolver resolves identically to the inline block in every case - fast present, base only, and MODEL unset/empty/explicit - checked against the old block on identical fixtures.
@TyroneNel

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (8b4dccd) — the branch had gone conflicting after #138 landed the validated resolver a few lines from the MODEL block, and after #134 added a docs/docker.md bullet next to the one this PR extends. You called that sequencing in your merge note; it just landed before the merge did.

Both conflicts were adjacent additions rather than competing edits:

No content changed. Re-run on the rebased tip rather than carried over: bench/test_model_verification.py, 2 tests / 12 cases, pass. git-apply and model-verification are both green and the PR is MERGEABLE / CLEAN, 6 files.

#136 is rebased on top and is now the single commit you asked for.

@mhenrichsen

Copy link
Copy Markdown
Contributor

Verified on the reference box tonight, merged onto today's main (this branch plus #136 on top, since #136 carries both of these commits — it merges with no conflicts after tonight's launcher changes):

bench/test_model_verification.py    Ran 2 tests ... OK   (Linux, the box's python3)
bash -n                              docker/entrypoint.sh, single-user/start_qwen.sh,
                                     single-user/select_model.sh, bench/warmup.sh, verify.sh: ok
select_model.sh, real model dirs:    MODEL unset -> .../Qwen3.8-27B-W4A16-AutoRound-fast
                                     MODEL=/some/explicit/path -> kept

And #125's first defect against the real files, with main's verify.sh --no-server: both the fast checkpoint (int4 lm_head) and the base (int8) pass all seven model checks, including #158's new head-group check that merged tonight. So the verifier half of #125 is already fixed on main; what this PR adds is the half that still bites — Docker verifying the base directory while the launcher serves the fast one.

The only thing holding the merge is on my side: the branch edits .github/workflows/patch-integrity.yml (it adds the model-verification job), and GitHub refuses a commit that touches workflow files from a token without the workflow scope, which mine lacks. That needs one command from the repo owner. The plan once it is added: merge this (it closes #125), then #136 on top of it. Nothing else is needed from you.

@mhenrichsen

Copy link
Copy Markdown
Contributor

Merging. Re-verified on top of the vLLM 0.29 pin (#148, merged just now) rather than on the old main: this branch plus #136 merged onto 858c3b7 with no conflicts, bench/test_model_verification.py OK, bash -n clean on all six scripts including both launchers, select_model.sh resolving to the fast checkpoint on the reference box's real model tree while keeping an explicit MODEL, and verify.sh --no-server 8/8 PASS on both the fast and base checkpoints. The workflow-scope block is lifted. Thanks for the patience on this one — it sat far longer than its review needed.

@mhenrichsen
mhenrichsen merged commit 25bd8d2 into syv-ai:main Sep 23, 2026
2 checks passed
cpuchip added a commit to cpuchip/qwen38-27b-rtx3090 that referenced this pull request Sep 23, 2026
…yv-ai#136) into main

Conflicts: docs/vllm-0.29.md takes syv's (a superset: the maintainer's greedy-divergence section); the image workflow
keeps this fork's identity (Dockerfile.fork into ghcr.io/cpuchip/...); PATCHES.md keeps this fork's header and
marlin-int8-asym-zp row, which describe the fork-exported file this main carries (hunks identical to syv's hand-cut
file; syv's series replays to fork tip 291980422 with 0 differing files). Batch GPU_UTIL 0.95 comes in from syv-ai#148.
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.

verify.sh rejects supported fast model and Docker verifies a different default model

2 participants