Repository navigation
qwen: share vision execution for 4B and 27B models - #117
Conversation
907ffbb to
6480b6b
Compare
hsliuustc0106
left a comment
There was a problem hiding this comment.
Reviewed at head 6480b6b90c (base 4a79980d; 12 files, +1327/−653; ~2,0xx authored lines, under the large-change threshold and proactively counted in the body). Verified in code — this is a careful, behavior-preserving extraction:
- The 4B path is pinned:
h == 1024branches call the original kernels with identical arguments,flash_kernel<64>keeps the exact legacy constants, and cua_s1 re-exports preprocessing throughImageLimits::cua()with byte-identical limits; cua_s1 keeps its own pinned checkpoint/LoRA validation. - The 27B path is additive under ABI 5:
ops.hstays atCS1_ABI_VERSION 5and the Rust side resolves the*_v2symbols asOption(verified atsrc/models/qwen3_5/native/src/cuda.rs:63-82) — an old ABI-5 library still loads for 4B, and 27B fails with an explicit "needs additive v2 CUDA symbols; rebuild libqwen3_5_cuda.so" error rather than silently degrading. Old-Rust/new-library also loads. - Ownership follows the architecture contracts: kernels under
src/backends/cuda/qwen3_5/, shared execution in the model crate, cua-specific policy stays in cua_s1.
No actionable findings.
Approving the code review; merge in stack order (#117 → #118 → #122 → #123), and note the finally-merged head should get a normal PR CI pass (the stacked PRs ran via workflow dispatch). Unverified: CUDA kernel behavior on hardware is author-reported (A800 campaigns) and was not re-run here — no GPU on this host; the rsqrtf(72) fast-reciprocal scale for head-72 is covered by those parity gates. Any merge of overlapping open PRs (#97–#108, esp. #103/#108) will force a rebase of this stack.
Provenance: canonical .agents/skills/system1-omni-review/SKILL.md (SHA-256 58be3bc6…dde108) and its repository map at trusted base 4a79980d8a75; remote head rechecked before posting.
hsliuustc0106
left a comment
There was a problem hiding this comment.
Incremental re-check at new head c15a98934b after the branch rebase (prior review: APPROVED at 6480b6b90c, 2026-10-08). Series identity: git range-diff of the previously reviewed head vs the new head shows every commit patch-identical and none added or dropped — a rebase, not new work. Provenance: review skill at main@4a79980d (SHA-256 58be3bc6…dde108); remote head re-pinned immediately before posting.
No new content, so the review verdict is unchanged: APPROVED, with the same unverified checks as before — the 27B CUDA parity and per-area GPU evidence remain author-reported (no GPU on this host), and no CI has reported on the rebased head yet (the identical series passed rust/build/benchmarks at the previously reviewed head). Merge in stack order (#117 → #118 → #122 → #123) still applies.
hsliuustc0106
left a comment
There was a problem hiding this comment.
Incremental re-check at new head 18094f16b3: one new commit since my approval at 1dc77639f5 — 900ad7a "qwen: delegate float_repr digits to serde_json for Python tie-to-even parity" — plus a merge. Verified: 900ad7a's json.rs blob is 95578103d6ce, byte-identical to the change exhaustively reviewed in my 2026-10-12 first-look (15/15 Python repr oracles at 3.12.8). This gives the previously-attributed stealth change its named home in this PR, resolving the provenance exception that #139's body has been tracking. Wave note: this pass also verified the rebase wave systematically — merge resolutions checked against both parents, zero dropped lines; CI exact-head as noted.
CI at this exact head: benchmarks/build pass. APPROVED (verdict carries; the newly-named commit's content was already oracle-verified). Transition note unchanged: add the post-#138 human answers and checklist when next touched. Unverified: GPU execution author-reported; Rust suite covered by exact-head CI.
…ne-20261011 cua-s1: keep vision features GPU-resident for language execution
FindingsP3 — 27B v2 kernels have zero in-repo test coverage P3 — third-party notices still attributed to the cua_s1 crate P3 — 4-entry FIFO scratch retention has no off switch |
hsliuustc0106
left a comment
There was a problem hiding this comment.
Incremental re-check at new head 502ff3d3ad: the branch merged the approved #135 stack (resident features + shared vision executor) — merge 502ff3d is a clean union (no conflict-resolution files at all), my approved 18094f16b3 commits are patch-identical (git range-diff, named refs, cat-file verified), and the absorbed #135 content is exactly what I re-approved at 8207cdf02 in the previous sweep (resident features intact, ABI 8 consistent, zero dropped lines — that verification carries). CI at this exact head: 4/4 pass.
APPROVED (verdict carries). Transition note unchanged: post-#138 human answers and checklist to be added when next touched. Unverified: GPU execution author-reported; Rust suite covered by exact-head CI.
hsliuustc0106
left a comment
There was a problem hiding this comment.
Incremental re-check at new head ddd7ab5f06 after the rebase wave onto main@e66ff34 (prior review: APPROVED at 502ff3d3a). Wave-verification note (this pass): the rebase wave was verified systematically — every own-commit series is patch-identical per range-diff; every post-pin merge resolution was diffed against both parents with zero dropped lines; approved-content markers verified present in every tree; the key new merge 3be46fa (shared-vision line × jemm v2 tip) carries #143\u2019s new test files intact and is ABI 8/8 consistent at every descendant head.
This branch: no merges of its own; the two new commits since the last pin (fb74c54 "cover 27B vision v2 numerics" + merge) were verified — fb74c54's licenses/notices/tests carried with zero delta, and the FIFO regressions plus the named 900ad7a float_repr commit remain in history. Fork/upstream CI green at this exact head.
APPROVED (verdict carries). Transition note unchanged: post-#138 human answers and checklist to be added when next touched. Unverified: GPU execution author-reported; Rust suite covered by exact-head CI.
|
Qualification gate: retain the existing HOLD The PR records exact-head A800 coverage for the v2 operators, 4B ordinary/resident vision, Graph paths and selected failure recovery. Those results should retain their recorded scope. Full current-source 27B/JEMM execution remains outstanding. Before releasing this hold, validate text and 1–4-image requests against the pinned reference, including changed image order, geometry changes, scratch growth, repeated reuse and error recovery. Review should follow configuration/tensor validation → preprocessing → device-view ownership → Graph lifetime → downstream model integration. Weight-free operators and 4B parity cannot substitute for full 27B qualification. No additional standalone correctness defect is asserted by this comment. Reviewed head: |
Conflicts resolved: - src/models/qwen3_5/native/Cargo.toml: keep both sides' [[test]] targets (our image_decode alongside upstream's device_features). - recipe/cua_s1/native_vision_graph.md: take the upstream wording and keep our clarifying sentence that vision Graph alone does not enable language Graph replay. Co-Authored-By: Claude Code <noreply@anthropic.com>
…ice binding InferMatrixLab#117 binds every DeviceBuffer to its allocation device: DeviceBuffer::new now reads the current device through the loaded CUDA library before any allocation, so a zero-byte buffer can no longer be constructed on a CUDA-free host and the two CPU-only offset tests cannot run. The assertion they pinned is carried by upstream itself (assert!(offset <= self.bytes) in DeviceBuffer::at), so no product behavior is lost; the test target and file are removed. Co-Authored-By: Claude Code <noreply@anthropic.com>
Main is at ABI 8 after InferMatrixLab#117, so the fixed-algorithm GEMM handle moves to ABI 9.
Brings in main with InferMatrixLab#117 as merged and InferMatrixLab#98 at ABI 9.
Brings in main at ABI 8 (InferMatrixLab#117) through InferMatrixLab#98, now at ABI 9.
# Conflicts: # src/backends/cuda/qwen3_5/qwen3_5.backend.json
Full 27B/JEMM ABI8 GPU qualification — 2026-10-21
HOLD: #117 remains open and unmerged. This is downstream full-model integration evidence executed at #118
5bdd625b4c27efb02f43a1f71150e1a7d7c0f071(ABI8), integrating #117 headddd7ab5f06b161f47d5b155abc0f9723a82ca85a. It is not a separate full-27B run directly at #117's own head. This result does not authorize a merge.Protocol, environment and measured result
One exclusive A800 80GB PCIe/sm80; driver 595.71.05; NVCC 13.0.88; cuDNN-enabled ABI8 build. Reference: Torch 2.12.1+cu130, Transformers 5.17.0, PEFT 0.21.0, FLA 0.5.2, Triton 3.7.1; cuDNN distribution 9.20.0.48 (runtime 92000). BF16 base/activations, separate FP32 rank-16/scale-2 LoRA (992 tensors); TF32 override 0, matmul precision highest, serial requests. The active GDN closure uses
fla.ops.gated_delta_rule.chunk.chunk_gated_delta_rule; no upstream inference patch or tolerance relaxation was used. All 577 tracked source files were verified before/after execution; CUDA/Rust builds used isolated fresh outputs.Official reference, native eager (Graph off) and native Graph (text/multimodal-language and vision Graph on) each completed two full passes of 28 requests / 49 questions, including the original frozen 16-request/25-question subset plus 12 extensions. Raw totals: 187 POSTs = 183 HTTP 200 + 4 expected HTTP 422 (reference 58, eager 62, Graph 62, frontend 5). This includes feasibility, full passes, invalid/recovery and frontend spots; no HTTP retries or extra full passes.
usage.latency_ms; raw latency is retained. Repeated identical inputs remain semantically stable.Public raw evidence and reproduction
Report and evidence release · unchanged raw archive · frozen protocol · independent local raw recomputation · SHA256 inventory
Raw archive SHA256:
6d068edc18fb377a6af402db1a4a2ad5cb04ca2891def10b76ea70420e95b395(7,077,244 bytes). It contains 90 manifest-listed evidence files, full raw request/response streams and server logs, bootstrap failures, provenance/build checks and the exact source archive. All 90 evidence records and all 577 nested source files passed local hash verification. Download every release asset and runpublic_audit.py(Python 3.9+): it verifies assets and reruns the original offline numerical/HTTP/Graph audit without a GPU. These are author-run hardware tests plus a separate local raw recomputation, not a maintainer GPU rerun.Pinned inputs: base
Qwen/Qwen3.8-27B@1d4bf0f2ff6012fd82039f2fa52739d0dd7c60c0; adapterMaestroYan/JEMM@76e3c209e8441fa658221c7ba2725bad2f811176; officialypcypc/JEMM@6822fe0fd53c5e6670af6ba99fb2c857a661e532. The immutable v2 export/cache was reused only after inventory verification. Startup helper/inventory mistakes and both reference bootstrap failures remain archived; all preceded qualification HTTP, and the successful operators/head were not rerun or gates changed to obtain a pass.Shutdown observations: after local download/hash/raw verification, platform shutdown disconnected SSH; a fresh connection worked before shutdown and a new connection was closed afterward. No platform stdout receipt or billing API confirmation was recorded. Private connection details are redacted. No server restart was needed for publication.
Scope and historical version boundaries
The native multimodal run uses host image embeddings (
vision.forward_images→forward_multimodal). It does not newly qualify full-27B resident-device embeddings, #127 cache increments, #143's separate precision profile, other accelerators, labelled accuracy or any throughput/latency/peak-memory measurement.All earlier sections below are preserved verbatim with their original version markers. The 2026-10-11 full JEMM A800 results remain ABI6 (original 16/25 corpus, multimodal language eager); the later 4090 results remain 11-operator-only; the 2026-10-15 #117 A800/Torch 2.14.0/ABI8 results remain operator/full-4B evidence. Earlier “full current-source 27B pending” statements are superseded only for the new scope measured above. Existing contributor declarations and maintainer verdicts are unchanged. #117 remains HOLD and unmerged; review order stays #117 → #118 → #122 → #123.
Current-head GPU verification — 2026-10-15
Exact tested head:
ddd7ab5f06b161f47d5b155abc0f9723a82ca85a.Merge hold remains in force: #117 stays open and unmerged.
Current-head GPU validation completed: 16 passing test executions across 9 groups. This includes the direct 1152-wide position/head-72 rotary/attention v2 oracles (original exact/0.004 gates), ordinary/resident vision Graph, finite scan, and full 4B host/resident feature/hidden/logit/probability/response parity at vision Graph 0/1. Changed pixels/text, geometry and scratch growth, nonfinite/input recovery, capture fallback and queued-copy drain were exercised using maintained synthetic RGB fixtures and the canonical pinned screenshot export.
The v2 operator oracles are weight-free. Complete 27B/JEMM checkpoint validation remains outstanding. #127's exact-head cache/resident/error-release run also passed as documented there; its results do not remove this PR's explicit hold.
Environment: A800 80GB/sm80, CUDA 13.0.88, Torch 2.14.0+cu130, Transformers 5.17.0, ABI 8. Four source archives and every archived source file were verified; no production-source edits or numerical-gate reductions were used. Report, raw responses/logs, protocols and SHA256 inventory are public. The downloaded archive and all 208 files were verified locally before platform shutdown; a fresh SSH probe was unavailable afterward. Independent local raw-evidence review is recorded with the release.
Initial dependency/path fixture failures and the disqualified stale shared-Cargo-target #127 attempt remain in the archive. The valid #127 and #143 runs use source-specific target directories. No throughput, latency, peak-memory, Hopper, full 27B checkpoint, FLA-enabled parity or real GUI task-quality claim is added.
Historical records below keep their named sources and environments. Pending-GPU statements in those earlier records are superseded only within the scope verified above. Existing contributor declarations and maintainer verdicts are preserved. No PR was merged.
CPU maintenance and merge handoff — 2026-10-15
Current head:
ddd7ab5f06b161f47d5b155abc0f9723a82ca85a.Actual target:
mainatf2d3d32897c1fe190fecf853a82a9456d3791462; the PR target was not changed.Corrected the shared CUDA contract's stale current-ABI-4 statement: use the backend header, manifest and Rust loader as the current authority (all three are 8 here). Documented the existing CPU CI metadata checks, marked the old proposal background and schema example as historical/illustrative, and kept CUDA compilation/device qualification separate.
Merge hold remains in force: keep #117 open and unmerged. The named float-repr provenance, notices and v2 regression code are already present. Remaining device work includes the current v2 operator oracles, Cua eager/Graph paths and downstream device/lifetime qualification. New GPU tests were not executed in this CPU-only maintenance.
Personally ran strict MkDocs,
git diff --check, 68 CUDA checker tests and 8 backend tests; all passed. The metadata checker reported zero errors and the existing Laya ABI warning. Header/manifest/loader all report ABI 8.This pass follows the contributor's 2026-10-15 instruction to maintain these existing PRs, complete CPU-only fixes and merge preparation, and keep #117 unmerged. Existing contributor declarations are preserved; this update adds no new human attestation or maintainer approval. No PR was merged and no GPU/model/performance run was performed. Source-pinned historical results remain historical. Current-head hosted workflows: CI run 38667376431, Docs run 38667376364. Read their live results separately from the personally executed checks above; these CPU workflows do not execute the skipped GPU tests.
Historical records below
Earlier maintenance/review/test sections retain their named revisions and environments. Their old ABI or build commands are historical reproduction instructions, not commands for deploying this current head. Use the current branch recipes for builds; the CPU checks above do not qualify skipped device tests.
Historical maintenance — 2026-10-15
Current head:
fb74c54b17c4a05fd047117a83119571c15be416.Addressed the three new P3 follow-ups: added direct 1152-wide position, head-72 rotary and attention regression oracles (including strided V, changed input and dirty padded workspace); colocated all preprocessing/vision notices and complete license texts with the shared Qwen crate; documented generic four-grid FIFO versus the separate explicit Cua count/byte LRU policy being integrated in #127. The fixed 0.004 attention gate and exact dyadic BF16 position/rotary assertions have not been exercised on a GPU in this maintenance run. These changes add coverage and deployment/ownership clarity without changing production model arithmetic.
Personally run: format, strict workspace Clippy, workspace tests (199 passed, 0 failed, 46 ignored), release build, benchmark tests, CUDA checker/backend tests, source contract check, smoke validation and strict MkDocs all passed. Device-dependent tests were compiled and skipped on this macOS host. No new GPU, numerical-parity or performance pass is claimed.
Current-head hosted workflows: ci.yml, docs.yml. Linked hosted results are separate from personally executed local checks. Earlier evidence sections retain their named original revisions and scope; they are not new qualification claims.
No PR merge was performed. #117 remains on its explicit merge hold.
Required human contributor answers
These answers reuse the contributor's explicit 2026-10-12 maintenance instruction:
keep the existing PRs mergeable and CI passing; eliminate conflicts and review
issues; serve maintainers who need clear, verified changes that are easy to merge;
repair, validate and push to the existing PRs. The source is the contributor's
local maintenance conversation on that date; private conversation details are
not published.
What outcome are you trying to achieve?
Keep the existing PRs mergeable and CI passing.
What is the highest-leverage variable for that outcome?
Eliminate merge conflicts and resolve review issues.
How will this work move that variable rather than create busywork?
Repair the identified conflicts and review issues, validate the affected
integration, and push the verified changes to the existing PR branches.
Who is the customer or user, and what do they actually need?
Repository maintainers, who need clear, verified changes that are easy to merge.
After answering the four questions: is a PR needed at all to achieve this outcome?
The contributor explicitly directed updates to the existing PRs. Maintain these
PRs; no new PR is needed for the maintenance work.
Observed follow-up evidence: the old language-test filter selected six cases, including the vision module and wrapper-only cases, instead of the runner's required three; the runner failed. The module/filter repair separates ordinary vision and language suites and preserves all tests. See the namespace correction and the maintained runner. The inherited float-format mismatch (
1000000000000000.2rendered as.3) is covered by the named fix/regressions. The latest18094f16merge adds named provenance with zero file-content changes.Purpose
Share Qwen RGB8 preprocessing and vision execution between the existing Cua 4B wrapper and the 27B layout. Configuration/tensor validation and optional dimension-aware CUDA vision endpoints stay in the shared executor; Cua retains its checkpoint/adapter contract.
The executor preserves vision Graph replay and four exact
[T,H,W]FIFO entries. Eviction synchronizes and destroys the Graph before its buffers. The maintenance integration also preserves main's text/multimodal language Graph controls, fixes Python-compatible shortest-float formatting, and isolates ordinary vision tests from language and fault-injection test filters.Historical Review scope
System1-Omni Version / Commit:
18094f16b342d29c5442838983f8a8dd60783f2f(history-only follow-up; source tree identical to qualified1dc77639).Integrated main baseline:
3e9cc33a61ddf110d97512fb6f296cf540377c61. Review order: #117 → #118 → #122 → #123. Review configuration/tensor validation, preprocessing/geometry, device layouts/lifetimes, Cua delegation, then root-tree regressions. Published history is preserved through normal merges.Historical Test Plan
System1-Omni Version / Commit:
18094f16b342d29c5442838983f8a8dd60783f2f.Reproduction commands from the repository root; recorded local feature switches and device prerequisites remain scoped to the original results above. This PR-body update runs no additional source or device checks.
cargo fmt --all --check cargo clippy --workspace --locked --all-targets --features omni-laya/serve -- -D warnings cargo test --workspace --locked --features omni-laya/serve cargo build --workspace --release --locked --features omni-laya/serve python3 -m mkdocs build --strictWith the pinned Cua 4B artifacts and rebuilt ABI 6 library exported as the documented
CUA_S1_*variables:cargo test --release --locked -p omni-qwen3-5-native --lib vision_graph_ -- --ignored --test-threads=1 --nocapture python3 tests/qwen3_5/run_graph_regressions.py --log qwen117-language-graph.logThe language runner requires
CUA_S1_TEST_MODELandCUA_S1_CUDA_LIB. Ordinary vision, language and injected-capture cases use separate filters/environments. This history-only head has the identical full tree of source-qualified1dc77639; no new GPU run is attributed to the ancestry merge.Historical Test Result
Local CPU checks: formatting, strict workspace/all-target Clippy, locked workspace tests and release build, benchmark helpers, and strict MkDocs. The recorded workspace run passed 190 unit/integration tests plus one compile doctest, with 36 device/checkpoint tests ignored. The neutral test-module follow-up passed fresh Clippy and CPU-only listings: two ordinary vision tests and three language Graph tests are selected separately; fault-wrapper tests remain separate.
RTX 4090 evidence from this maintenance campaign: twelve checkpoint-free CUDA kernel regressions passed after the recorded setup correction. Two ordinary vision Graph regressions, three language Graph regressions and capture-failure checks also passed before the final namespace-only revision. Final
1dc77639passed fresh two ordinary vision and three language Graph regressions, with exactly one unconditional injected-failure diagnostic in each language mode. The first final-language attempt lacked CUA_S1_TEST_MODEL and failed before device inference; the corrected explicit environment passes. Kernel implementation is unchanged from the twelve-case run above. Setup failures and corrected-run records are retained. The source-qualified predecessor1dc77639passed upstream CI/Docs; new-head hosted status is recorded separately below.The 27B vision layout, normalized checkpoint loader and generalized preprocessing are not established by the Cua 4B tests. No full 27B checkpoint qualification or new performance claim is made. CPU/hosted checks do not execute ignored GPU tests.
Demo / evidence
Historical A800 evidence remains scoped to its recorded source/library/workloads. Old hosted workflow results are not verification of this maintenance head; current-head hosted status must come from checks attached to this commit.
The required loader ABI remains 6; optional 27B vision v2 symbols require a rebuilt library. Four-grid retention bounds entry count, not total bytes; weights, GEMM plans and driver overhead are additional. The cache does not add concurrent forwards or dynamic batching.
Agent-assisted component and integration review prepares this contribution; contributor/maintainer review and dependency merges remain separate.
Named float_repr provenance — 2026-10-13
Per the provenance request, normal merge
18094f16incorporates named fix900ad7a. Both changed files already matched. Staged and committed full-tree comparisons against prior head1dc77639are empty; complete tree remains29235c319ed4d3bef79b602c48e866145b05bb16. Named fix ancestry is verified. This adds exactly the named commit and one merge commit, with no rebase, force push or file-content change.Prior CPU/GPU source evidence retains its recorded provenance and applies to the identical tree; no fresh GPU run is claimed for the history-only update. New-head CI and Docs passed. PR #117 remains open and unmerged; naming the inherited change does not replace maintainer review or outstanding 27B validation.
Video: N/A for this nonvisual API, validator, shared executor or documentation change. Source-pinned tests and the linked validation records provide the relevant evidence; existing hardware and unverified-measurement limits remain unchanged.
Self-review
The contributor explicitly confirmed completion of self-review and authorized this checklist/template update on 2026-10-13 in the maintenance conversation. This records the human confirmation; it does not turn unverified measurements into passes or replace maintainer review.
Merge hold: PR #117 remains open and unmerged under the contributor's explicit 2026-10-13 instruction. Template completion and CI success do not authorize a merge.