Repository navigation
Conversation
3601009 to
ef66b15
Compare
cuBLASLt's heuristic picks an algorithm per M, so a row's result can change with the number of rows in the call, and a prefix and its branch run separately round differently from one pass over the same tokens. Add cs1_gemm_create_fixed: one algorithm per weight shape (N, K, ldy), the heuristic's first choice at a reference M among algorithms without split-K, for every M. The default handle is unchanged. Bump the ABI to 7 and add a GPU test that each projection's rows match across M.
forward_shared captures each prefix with the prefix continuation from InferMatrixLab#96 and continues every branch from it, with the fixed-algorithm GEMM handle; forward_fixed is the same path without a prefix. run_window takes a GEMM handle, can capture after a continuation, and indexes the text rotary tables by position.
ef66b15 to
2ff1396
Compare
… run_window) for the worker
# Conflicts: # src/models/qwen3_5/native/src/model.rs
|
Integration note: reject unsupported execution profiles before device work
Preserve the prefix-state, scratch-growth and Graph-shape guards during conflict resolution. Add a CPU regression that proves early rejection of unsupported profiles. Device qualification should cover merged 4B and 9B, Graph off/on, reordered groups/branches, growth, ordinary-forward interleaving and recovery after invalid input. Shared-prefix output must match a full pass using the same fixed handle exactly. Any comparison with the default handle’s different arithmetic should remain a separately reported metric. Reviewed head: |
|
Agreed. Main has no unmerged or reference profile yet, so if #118 or #143 lands first I'll make |
|
@twu3202 Dependency update for the execution-profile guard we discussed: #117 is now in When integrating the unmerged/reference profiles from #118/#143, keep the agreed explicit rejection in #118 is marked merged into |
Main is at ABI 8 after InferMatrixLab#117, so the fixed-algorithm GEMM handle moves to ABI 9.
Brings in main at ABI 8 (InferMatrixLab#117) through InferMatrixLab#98, now at ABI 9.
Brings in InferMatrixLab#155. Its reference GEMM handle keeps every reduction scheme next to the fixed handle, and its fake test library reports ABI 9.
Brings in main with InferMatrixLab#155 through InferMatrixLab#98. run_window keeps its prefix-state and shape checks, also refuses FP32 GDN, and applies the LoRA deltas on the GEMM handle it is given.
Both cover merged weights with the default numerics. A model with an unmerged LoRA adapter, reference numerics or FP32 GDN is now refused before any allocation or CUDA call. A CPU test loads a tiny checkpoint with an adapter on a fake CUDA library that counts calls, and checks that both entry points fail without a call or a buffer for each profile.
Brings in main with InferMatrixLab#155 through InferMatrixLab#98, InferMatrixLab#99 and InferMatrixLab#129. The profile check moves from forward_multimodal_readout into readout_window, which it now calls.
|
Refreshed after #98 at |
|
rebase |
Brings in InferMatrixLab#98 as merged (5ba6a3b) and InferMatrixLab#93. The one conflict is the backend README's prefix-continuation paragraph, which keeps this branch's sentence on forward_shared.
Brings in main with InferMatrixLab#98 as merged (5ba6a3b) and InferMatrixLab#93, through InferMatrixLab#99 and InferMatrixLab#129.
|
Done, merged |
|
Additional independent review at I traced 64-token prefix alignment, restored attention/GDN/conv state, branch order, fixed-GEMM profile rejection, scratch growth and request-local state ownership. Personally run: 24 CPU tests passed / 27 ignored. The two GPU prefix regressions then passed with Graph off and again with Graph on (four test executions), including regrouping/interleaving, invalid inputs, growth and exact equality with The generic GPU input was derived from already-cached The actual CUDA inputs were byte-identical to the independently compiled ABI9 library used here; all GPU commands used immediate nonblocking reservations. Strict docs passed. Cua-S1/Open-Jev trained exports, their task-level quality/parity and performance claims were not rerun. This is a validation comment, not a formal approval. Review provenance and scope
|
Levius-Fubuki
left a comment
Levius-Fubuki
left a comment
There was a problem hiding this comment.
Independent full review at f54b3cdf528c10d021a5980c5c87e9558fd4c0ce: no actionable findings. APPROVED.
Reviewed the complete eight-file increment, including the tests and documentation. I traced request/group/branch reconstruction, 64-token split boundaries, RoPE offsets, attention KV and convolution/GDN snapshot restoration, group/branch order, scratch growth and Graph invalidation, cached-state ownership, fixed GEMM creation/destruction, and synchronization on errors. The merged/default-numerics guard rejects unmerged LoRA, reference numerics and FP32 GDN before allocating shared resources or issuing device calls; the CPU fake-library regression exercises each guard.
The API documents its numerical contract accurately: shared execution matches forward_fixed only on hardware where the fixed algorithms preserve row independence. It does not promise bit equality to ordinary forward. The two reusable allocations retain capacity across calls, while prefix contents are regenerated within each call. No production worker is newly wired to this entry point in this PR.
Personally run on a clean archive of this exact head, macOS arm64, Rust 1.98.0:
cargo fmt --all --check: passed.cargo clippy --workspace --locked --all-targets --features omni-laya/serve -- -D warnings: passed.cargo test --workspace --locked --features omni-laya/serve: 259 passed, 0 failed, 53 ignored, including doctests. Ignored hardware/checkpoint cases were not executed.cargo build --workspace --release --locked --features omni-laya/serve: passed.python -m mkdocs build --strict: passed.- Full PR diff whitespace check: passed.
Observed hosted CI is separate from those local checks: exact-head CI 39800056074 and Docs 39800056087 succeeded; deploy was skipped. No unresolved inline review threads were present.
Current target main is f5ae5acf65e2d68150aef944aae00f4dffc0da27; diff merge-base is 5ba6a3ba668cb33028eb0bce2c6d17cdc53aebe7. A read-only merge preview is conflict-free, tree c9c598f87b32216461527a781e8d5746495e9c55. Its difference from the tested head consists of main's two Laya TileLang target files; no reviewed shared-executor source is altered by that preview. The preview itself was not used to claim a second full build/test run.
Scope/counts: 859 changed source/test lines, four manifest lines, 32 documentation lines; 895 total changed lines, eight files (+856/-39). All test bodies are under root tests/; new C fixtures are maintained test inputs, not one-off run output. Contributor answers, PR-necessity decision and all four self-review declarations are present.
Material limits: this local host has no NVIDIA CUDA device or the trained Cua/Open-Jev exports, so I did not rerun the two ignored prefix GPU cases, the fixed-GEMM device gate, model-level parity, or any performance campaign. The existing same-head GPU validation comment and author measurements remain separately attributed evidence; this approval does not re-certify those measurements or broader hardware/model coverage. Land this shared primitive before its #113/#129/#134 consumers and keep their integration gates.
Review provenance: repository ID 1386454853; trusted main@f5ae5acf .agents/skills/system1-omni-review/SKILL.md, SHA-256 ae2dfac116376ae2ec41f65122cc1bf5aea10751b8b907198a56154e5d67230e. Actually read its repository-contract reference, architecture, CONTRIBUTING, self-review artifact/demo rules, runtime contract and CI/Docs workflows. PR changes to documentation were reviewed as content. Independently serialized full-index diff SHA-256: 9b264a6b06114a049c1fa4e4b83134d5e6d43679b4c9e4caed9705962c665578 (git diff --no-ext-diff --no-textconv --no-color --full-index <merge-base> <head>). The remote head is rechecked before submission. No merge performed.
This branch has not been deployed
4 participants
Required human contributor answers
What outcome are you trying to achieve?
Run faster on native qwen3.5 workers for requests that ask several questions about the same thing.
What is the highest-leverage variable for that outcome?
How many prefix tokens each request prefills.
How will this work move that variable rather than create busywork?
qwen3_5: run prompts that share prefixes once per prefix #99 runs a shared prefix once and continues each branch from its state. qwen3_5: add a GEMM handle with one algorithm per weight shape #98 is the pre-req of qwen3_5: run prompts that share prefixes once per prefix #99, making a GEMM row's result no longer depends on how many rows are in the call.
Who is the customer or user, and what do they actually need?
agent asking several questions about one same thing such as screenshot, and they need faster answers.
After answering the four questions: is a PR needed at all to achieve this outcome?
Yes, to answer #85.
For a bug fix: link actual observed failure evidence.
N/A, not a bug fix.
Purpose
For #85, the shared executor step of the plan (§3; PR 3 in §7), as revised in this update. It builds on #96's prefix continuation and on #98's fixed-algorithm GEMM handle, both now in
main. This PR's own changes are2ff1396and866bed5(the profile check below); the merges bring in #98 andmain. No worker change; #113 builds on it for Decider.Model::forward_sharedruns prompts that share token prefixes: a request prefix, then groups with their own prefix and branches. Each prefix runs once, and each branch continues from the state after it, with positions and the causal mask continuing too. It returns one final hidden state per branch, in order.run_windowinto aPrefixState: per linear-attention layer the float32 recurrent state and the last three conv inputs, per full-attention layer the prefix keys and values. Each branch then runs as its own pass from that state. The request and group states persist across calls and grow only for a longer prefix; their contents are valid only within one call, which runs eagerly and synchronizes its stream before returning, also on error.forward_fixed, in an earlier version of the tests below that also ran exact splits. The result would also depend on the prompt's siblings, since they set the common prefix. Rounding down avoids both, at the cost of up to 63 extra tokens per branch.forward_fixed, a plain pass over the full prompt with the same handle and the samerun_window, bit for bit, however the prompts are grouped. It differs fromforward, which picks algorithms per row count, by rounding.run_window: it takes the GEMM handle; it can capture after a continuation, which a group prefix needs; and for text positions it offsets the rotary tables by the window start, which no caller reached before.forward_multimodal_captureandforward_multimodal_continuebehave as before, andforward,forward_batch,forward_multimodaland CUDA Graph replay are unchanged.forward_fixedandforward_sharedcover merged weights with the default numerics. Since Integrate JEMM native serving and Cua-S1 precision fixes into main #155,mainalso has unmerged LoRA adapters, reference numerics and FP32 GDN, so both refuse a model with any of them before any allocation or CUDA call.run_windowkeeps Integrate JEMM native serving and Cua-S1 precision fixes into main #155's LoRA deltas, now on the GEMM handle it's given.docs/architecture.md, the runtime README and the backend README now describe the entry point, its state and its extra memory.Test Plan
System1-Omni Version / Commit:
f54b3cd, a merge ofmain5ba6a3b(#98 as merged, and #93) into866bed5; against866bed5it only adds #93's benchmark files.866bed5is the profile check, on two merges of #98 into the earlier head18cbd99, the last of #98c2b1ab7(withmain32bdd2a).18cbd99mergedmain3e9cc33intocb8ddb3, a merge of #98d8cd04finto2ff1396;2ff1396is on #98d717e49, onmain0d25210. The timing ran at an earlier head; the other results below are from866bed5, and the CPU checks also pass onf54b3cd.CPU: the five Rust steps in CI (format, Clippy, frontend tests, workspace tests, release build),
mkdocs build --strict, a test of how prompts split into runs, and a test of the profile check: it loads a tiny checkpoint with an adapter on a stand-in CUDA library that counts calls, and checks that both entry points refuse the adapter, FP32 GDN and reference numerics, each on its own, with no call and no buffer.GPU, one RTX 6000 Ada (sm_89), on the Cua-S1 4B and Open-Jev-9B exports, with random token ids:
forward_sharedagainstforward_fixedon every full prompt: request prefixes of 0, 1, 63, 64, 65, 300 and 1,000 tokens, groups with 0, 1, 70 and 128 prefix tokens, and branches of 1 to 200 tokens;forwardbefore, between and after the shared runs; invalid requests (no groups, an empty group or branch, out-of-vocabulary ids anywhere) refused, after which the model still gives the same results;CUA_S1_GRAPH=1on 9B, where the plainforwardbetween shared runs replays a CUDA Graph;main, Cua-S1 and Open-Jev-9B workers side by side;Test Result
CPU. All six commands pass on macOS, including the split and profile tests.
forward_sharedagainstforward_fixed. Every branch is bit-identical, at every request prefix length and group shape above, on 4B and 9B, and on 9B withCUA_S1_GRAPH=1. Againstforward, the difference is that of the GEMM algorithms alone: up to 1.84e-2 (4B) and 1.53e-2 (9B) relative L2 on these random prompts. That is above the plan's 1e-2 hidden-state gate and inside #92's native vs reference spread (p99 0.023), so the update proposes how to treat that comparison.State isolation. Every check passes, and the model gives the same results after the refused requests. All of the invalid requests fail validation before any GPU work, so the error path's synchronization isn't exercised by these tests.
Kernel tests. 16 of 16 pass.
Plain forward unchanged: Cua-S1 567 of 567 byte-identical (134 of them forward passes) in eager and CUDA Graph modes, and Open-Jev-9B 253 of 253 identical apart from timing, against
main32bdd2a.Timing (9B, random tokens, one warm call each, one request prefix and 38-token branches):
forward_sharedforwardper branchA single pass with the fixed GEMMs takes 3% less time at 64 tokens, 8% more at 1,024 and 17% more at 3,072 than
forward; the prefix pass carries that cost once per request. These are synthetic layouts with random tokens. M1's real 3,093-token prefix would rerun 21 tokens per branch. The plan's measurement on real Open-Jev requests comes with the worker PR.Demo / evidence
No demo: no worker uses the entry point yet. Test output and scripts are kept locally.
Self-review