Skip to content

cua_s1: add a native CUDA text worker - #19

Merged
hsliuustc0106 merged 17 commits into
ThinkFlowLab:mainfrom
twu3202:cua-s1-native-pr
Sep 30, 2026
Merged

hsliuustc0106 merged 17 commits into
ThinkFlowLab:mainfrom
twu3202:cua-s1-native-pr

Conversation

@twu3202

@twu3202 twu3202 commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Part of #10: the minimal native text/prefill path for Cua-S1 4B 0.2, as discussed on #12. It builds on #13, whose commits are included here until it merges; the #13 worker is the correctness reference it is checked against.

It serves the text adapter on /v1/systemone like the #13 worker (validation, status codes, prompt token ids, answer format) and runs the forward pass on its own CUDA kernels, without Python or PyTorch:

  • src/models/cua_s1/native/: the Rust crate omni-cua-s1-native: request handling, tokenization, the Qwen3.5-4B forward pass (weights, buffers, the layer loop) and the 26-row head.
  • src/backends/cuda/qwen3_5/: the operations in CUDA C++: RMSNorm variants, the Gated DeltaNet convolution, gates and chunked prefill, rotary embedding, a FlashAttention-2 style attention kernel on tensor cores, and bfloat16 GEMMs through cuBLASLt. build.sh builds libqwen3_5_cuda.so, which the worker loads at run time, so the workspace builds and tests without a CUDA toolkit.
  • recipe/cua_s1/native.md and export_text_merged.py: build, export the merged weights, launch.

Choices worth a look:

The comparison scripts behind the results below are not part of the change. They are kept at f9ab3fc.

Test Plan

  • CI's checks with Rust 1.98.1: cargo fmt --all --check, cargo clippy --workspace --locked --all-targets -- -D warnings, cargo test --workspace --locked and cargo build --workspace --release --locked.
  • Kernel tests on the GPU (CUA_S1_CUDA_LIB=$PWD/target/release/libqwen3_5_cuda.so cargo test --release -p omni-cua-s1-native --test kernels -- --ignored): attention at 1 to 2,048 tokens and the chunked Gated DeltaNet prefill, each against a float64 reference.
  • Accuracy: the fixed input set from cua_s1: add a text worker that loads Qwen3.5-4B through Transformers #13 (14 requests, 16 questions) sent to this worker and checked against the float32 results of the cua_s1: add a text worker that loads Qwen3.5-4B through Transformers #13 worker.
  • The previous version of this PR, run eagerly, and this one on the same 46 requests (the input set, a case for every error).
  • The export script run again, and its output compared file by file with the export used here.
  • Latency: bench_text.py over HTTP, one request at a time, 3 warmup and 20 measured requests per case, two runs each.

Environment: one RTX 6000 Ada (48 GB, sm_89), CUDA 13.2, driver 595.91.07, Rust 1.98.1, with the pinned revisions.

System1-Omni Version / Commit: 8367333 on #13 (cfbccc4).

Test Result

  • CI's checks pass. Both kernel tests pass: attention is within 4.9e-3 of the float64 reference relative to each row's largest value, and the Gated DeltaNet prefill within 5.8e-4.
  • Accuracy: the largest difference from the float32 worker is 0.0057 over the 16 questions, against the allowance of 0.039, and no top option changes.
  • Against the previous version run eagerly, every probability is identical (one confidence differs in the last digit, from summing in a different order), and every error gets the same status, except that a body over 4 MiB now gets its 413 where the previous version reset the connection.
  • The export script writes files byte-identical to the export used here.
  • Startup: 1.9 s to a ready /health; the first request after that took 14 ms. The card peaked at 12.1 GiB in use while serving.
  • Latency, p50 in milliseconds. The cua_s1: add a text worker that loads Qwen3.5-4B through Transformers #13 column is from cua_s1: add a text worker that loads Qwen3.5-4B through Transformers #13's description; cua_s1: add a text worker that loads Qwen3.5-4B through Transformers #13 computes logits at every position, which accounts for part of the gap:
Prompt tokens #13 This PR
139 46.2 18.2
218 49.2 24.8
292 52.7 32.5
712 109.3 68.8
15,446 3084.7 1588.5

Not covered here: score and noul questions and the multimodal adapter; more than one request at a time; GPUs other than sm_89 and CUDA versions other than 13.2.

Self-review

Before marking this PR ready for review or requesting maintainer review, complete
the self-review checklist.
Keep the PR in draft while this work is incomplete.
For agent assistance, use the optional precheck-pr skill.

  • I have reviewed the full diff and addressed the issues I found.
  • I have checked that the change follows the project's architecture and stays focused on the stated purpose.
  • I have run the checks appropriate to this change and reported commands, results, and anything I could not verify above.
  • I have checked that the PR description, documentation, and any accuracy or performance claims match the implementation and available evidence.

Add a /v1/systemone worker for the Cua-S1 4B 0.2 text adapter in
src/models/cua_s1/text/, next to the multimodal worker proposed in ThinkFlowLab#12.
It loads the base model and the PEFT adapter directly through
Transformers and PEFT, follows the contract in
src/models/cua_s1/README.md, and answers choice questions only.

Add the fixed input set and tests in tests/cua_s1/ (contract and HTTP
tests that need no weights, and tokenizer checks), and
recipe/cua_s1/text.md with setup, launch, a parity check against
upstream FourBModel and a latency script. Ignore the recipe's weights/
and .venv/ with the same .gitignore lines as ThinkFlowLab#12.

Part of ThinkFlowLab#10.

Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
FastAPI runs a returned dict through jsonable_encoder, which drops every
key that starts with "_sa". Question names and option keys come from the
request, so a question named "_sample" or an option named "_save" was
missing from the answer while its tokens still counted in usage, and the
choice could name an option that was not in the probabilities.

Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
Resolve the .gitignore and recipe/README.md conflicts with the frontend
merge (ThinkFlowLab#2): use the same Python ignores as ThinkFlowLab#12, and list the Cua-S1 text
recipe next to the Laya one.

Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
The frontend is merged (ThinkFlowLab#2), so the recipe builds it from the repository
root instead of the pull request branch.

Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
A /v1/systemone worker for the text adapter in Rust
(src/models/cua_s1/native, crate omni-cua-s1-native). It answers every
request as the reference worker in src/models/cua_s1/text does, with the
same validation, error bodies, prompt token ids and answer format, and
runs the Qwen3.5-4B forward pass on its own CUDA kernels in
src/backends/cuda/qwen3_5.

The kernels are built into libqwen3_5_cuda.so by build.sh in that
directory and loaded at run time, so building the workspace needs no
CUDA toolkit. The adapter is merged into the bfloat16 weights
beforehand, by recipe/cua_s1/export_text_merged.py. Prompts up to 2048
tokens run as CUDA graphs captured per exact length, bitwise identical
to the eager pass. GEMMs go through cuBLASLt with algorithms tuned per
GPU and kept in a file that records the GPU and cuBLASLt version they
were tuned for. Attention and the chunked Gated DeltaNet prefill run on
tensor cores.

recipe/cua_s1/native.md covers the build, the export, launching, and the
checks against the float32 reference worker and against the reference
worker over HTTP.

Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
xiaoyu-xyz added a commit to xiaoyu-xyz/system1-omni that referenced this pull request Sep 28, 2026
CUDA build paths are now in flight and diverging: Laya generates CUDA from
TileLang, Cua-S1 hand-writes CUDA C++ with cuBLASLt, and the multimodal worker
plans Triton. Nothing links against anything else yet, so the divergence is
invisible today and blocking as soon as one model reuses another's kernels —
which ThinkFlowLab#9 already plans, since a Kev engine and Cua-S1 share a Qwen3.5 backbone.

This adds the contract and the checker, and nothing else:

- contract.md: what backends must agree on — a JSON manifest per backend, four
  C ABI rules, the numerics that must be declared rather than discovered in a
  parity failure, and the build-script interface.
- check_contract.py: reads the manifests and checks them against that contract.
- build_script.py: reads a build script as text and reports what it declares.
  Never executed, because CI must not run repository code to decide whether a
  manifest is honest.

A backend may be a subdirectory named after itself or sit directly under
src/backends/cuda/, which is the layout Laya's kernels/ and tools/ use. The
repository root is passed down explicitly rather than inferred by walking up from
the backend, because the flat layout makes that guess one level wrong.

The compile and parity tiers need hardware and are described in contract.md but
deliberately not wired up here. This change needs no CUDA toolkit and no GPU,
which is why it can land ahead of them.

53 tests pass. The checker was run against the real qwen3_5/ files from ThinkFlowLab#19,
which it accepts, and against the real tools/ from ThinkFlowLab#16, which it rejects until
that backend exposes an executable build script — the divergence it exists to
surface.
@xiaoyu-xyz

Copy link
Copy Markdown

Read src/backends/cuda/qwen3_5/ — I'll focus these comments on the CUDA side, since that's the part I'm looking at. Starting with gemm.cu; attention.cu and gdn_prefill.cu next.

What is right, and it is not obvious

  • In-place split-K reductions are excluded because their accumulation order, and so the rounding, is not fixed. Giving up some speed for a reproducible result is the right call for an engine that will be checked against a reference, and reduces_in_place re-checks on import rather than trusting the file.
  • cs1_gemm_import is all-or-nothing. Every plan is described and checked into checked before any of them replaces a live plan, so a bad file changes nothing.
  • The 3 % threshold with "near ties rarely change between runs" is the honest way to handle this: it admits the measurement is noisy instead of chasing it.
  • Timing has an L2 flush before each call and takes a median, and a first pass of one call each narrows many candidates to the twelve worth timing properly.
  • static_assert(sizeof(cublasLtMatmulAlgo_t) == sizeof(out[n].algo)) pins the ABI assumption at compile time rather than hoping.

Three things I would change

1. plan_for's fallback can adopt an algorithm tuned for a very different M.

if (above && usable(g, p, above->algo)) { p.algo = above->algo; }
else if (below && usable(g, p, below->algo)) { p.algo = below->algo; }

usable() asks whether the algorithm runs for this shape, not whether it is good for it. "The smallest tuned M above, else the largest below" means a shape at M=4096 can end up on an algorithm tuned at M=128 when nothing above has been tuned, with no heuristic fallback and no measurement. The comment describes the selection but not that it is unvalidated — worth either bounding how far the borrowed M may be (say a factor of two) or falling back to the heuristic outside that band.

2. flush_l2 is likely to be optimised away, so the L2 flush before each timed call may not happen.

int acc = 0;
for (...) acc ^= p[i].x ^ p[i].w;
if (acc == 0x7fffffff) *sink = acc;

The store is conditional on a value that essentially never occurs, so the compiler can prove the loop has no observable effect and delete it — at which point g->flush is only ever touched by the cudaMemsetAsync at allocation, and no flush happens inside the timing loop at all. It would not be a correctness problem (this only tunes), but it would make the file's own claim, "times cuBLASLt's candidates for a shape with L2 flushed before every call", untrue, and it would bias the search toward whichever candidate ran first. A store of acc to a sink that is always written, or asm volatile, removes the doubt.

3. Cs1GemmPlan does not carry the cuBLASLt version, and the comment hands that to the caller.

cs1_gemm_version() exists and is exported, but nothing ties the value to the plans. The header says the caller keeps that with the plans, which means the failure mode of forgetting is a silently different (or wrong) algorithm rather than an error. Adding a version field to the struct would be an ABI change, which is what CS1_ABI_VERSION is for — worth doing while the only callers are in this repository.

Smaller notes:

  • every_config returns an empty vector when cublasLtMatmulAlgoGetIds fails, which makes cs1_gemm_tune(..., exhaustive=1) silently fall back to the heuristic's shortlist. Returning an error, or logging, would make that visible.
  • The log_choice static const bool on = std::getenv(...) is initialised once, so CUA_S1_GEMM_LOG cannot be toggled after the first tuned shape. Fine, just worth knowing.

I have not run any of this — these are from reading, and the two that I would act on first are 1 and 2.

xiaoyu-xyz added a commit to xiaoyu-xyz/system1-omni that referenced this pull request Sep 28, 2026
Both models that need this readout share the shape of it: Kev (ThinkFlowLab#27) scores
scale * dot(k_proj(c_i), q_proj(s)) and softmaxes over a question's candidates;
CLM (ThinkFlowLab#9) scores exp(logit_scale) * cos(state_head(s), action_head(c)) and does
the same. ThinkFlowLab#9 asks for exactly this fusion, and ThinkFlowLab#19's gemm.cu does not have it.

The fusion is more than fewer launches. For a cosine the candidate's norm and its
dot with the query need the same elements, so both accumulate in one read of C --
for Kev, 255 rows of 2560 floats read once instead of twice. The softmax then
runs in-block over shared memory, so there is no second kernel, no atomics and no
global round trip for the similarities. A batch is a grid over questions rather
than a loop of launches.

candidate_scoring.cu has NEVER BEEN COMPILED. The authoring machine has no CUDA
toolkit and no NVIDIA GPU, so nothing here claims it compiles, runs or is fast.
What is established is the arithmetic it performs:

- reference.py is the float64 oracle, and its invariants are tested: a
  distribution, 1/K for indistinguishable candidates, no NaN for a zero
  candidate, no overflow where a naive softmax would produce inf.
- kernel_simulation.py reproduces the kernel's algorithm in numpy -- float32
  accumulation, the max-subtracted softmax, the norm floored as
  max(sqrt(sum), eps) and not sqrt(max(sum, eps)) -- and agrees with the oracle
  to a worst absolute difference of 1.8e-07 over the fixed cases. The simulation
  rounds twice per multiply-add where the kernel uses fmaf, so that is an upper
  bound on the kernel's error.
- The reduction is a tree, matching __shfl_down_sync, tested structurally and
  shown to differ from a sequential sum on a concrete input.

Tolerance is declared at 1e-4, three orders of magnitude above the simulated
worst case, so a hardware failure is a failure rather than tolerance noise.
gpu_parity.py checks a compiled library against the reference and skips with a
reason when there is no GPU or no library.

The inputs are not committed: they are deterministic from a fixed seed and would
be about a megabyte of generated floats.

17 tests pass. The backend satisfies the contract from ThinkFlowLab#25, which caught the
manifest over-claiming architectures the build script does not reach by default.
xiaoyu-xyz added a commit to xiaoyu-xyz/system1-omni that referenced this pull request Sep 28, 2026
Both models that need this readout share the shape of it: Kev (ThinkFlowLab#27) scores
scale * dot(k_proj(c_i), q_proj(s)) and softmaxes over a question's candidates;
CLM (ThinkFlowLab#9) scores exp(logit_scale) * cos(state_head(s), action_head(c)) and does
the same. ThinkFlowLab#9 asks for exactly this fusion and ThinkFlowLab#19's gemm.cu does not have it.

The fusion is more than fewer launches. For a cosine the candidate's norm and its
dot with the query need the same elements, so both accumulate in one read of C --
for Kev, 255 rows of 2560 floats read once instead of twice. The softmax runs
in-block over shared memory, so there is no second kernel, no atomics and no
global round trip for the similarities. A batch is a grid over questions.

MEASURED, not just intended. RTX 4090 (sm_89), driver 595.58.03, CUDA 13.0
(V13.0.88):

    worst absolute difference: 1.835e-07 (tolerance 0.0001)
    all cases within tolerance

All 13 fixed cases pass, including a single candidate, identical candidates, a
zero candidate, K=255, and logits whose raw exponential overflows.

Three defects were found and fixed; the first two only a GPU could show.

1. The parity harness passed host pointers where device pointers were expected,
   so the kernel dereferenced host memory as device memory. compute-sanitizer
   located it as an invalid global read with consecutive lanes on consecutive
   addresses. Fixed by allocating, copying and synchronising through cudart.

2. The query norm was summed across warps that had each computed the same
   partial: the inner loop already covers all of D within one warp, so the norm
   came out sqrt(8) too large on a 256-thread block. Every unnormalized case sat
   at float32 rounding while the normalize cases were off by ~1e-2, which is what
   pointed at it. Warp 0 now computes it alone.

   kernel_simulation.py did NOT catch this, because it was written from the
   intent rather than transcribed from the .cu. A simulation is only as good as
   its fidelity; hardware is what settles it.

3. The manifest claimed sm_89 while build.sh defaults to sm_80. The contract
   checker from ThinkFlowLab#25 caught that, as it caught the same class of over-claim in the
   Laya backend.

The manifest is now status=validated with a declared tolerance and a reference
entrypoint, which is what the contract requires of a backend making a parity
claim. The reference is float64 in reference.py, and gpu_parity.py skips with a
reason rather than passing when there is no GPU or no library.

Not yet exercised: the batch path beyond questions=1, the K>255 rejection, and
any architecture or CUDA version other than sm_89 on 13.0.
xiaoyu-xyz added a commit to xiaoyu-xyz/system1-omni that referenced this pull request Sep 28, 2026
Both models that need this readout share the shape of it: Kev (ThinkFlowLab#27) scores
scale * dot(k_proj(c_i), q_proj(s)) and softmaxes over a question's candidates;
CLM (ThinkFlowLab#9) scores exp(logit_scale) * cos(state_head(s), action_head(c)) and does
the same. ThinkFlowLab#9 asks for exactly this fusion and ThinkFlowLab#19's gemm.cu does not have it.

The fusion is more than fewer launches. For a cosine the candidate's norm and its
dot with the query need the same elements, so both accumulate in one read of C --
for Kev, 255 rows of 2560 floats read once instead of twice. The softmax runs
in-block over shared memory, so there is no second kernel, no atomics and no
global round trip for the similarities. A batch is a grid over questions.

MEASURED, not just intended. RTX 4090 (sm_89), driver 595.58.03, CUDA 13.0
(V13.0.88):

    worst absolute difference: 1.835e-07 (tolerance 0.0001)
    all cases within tolerance

All 13 fixed cases pass, including a single candidate, identical candidates, a
zero candidate, K=255, and logits whose raw exponential overflows.

Three defects were found and fixed; the first two only a GPU could show.

1. The parity harness passed host pointers where device pointers were expected,
   so the kernel dereferenced host memory as device memory. compute-sanitizer
   located it as an invalid global read with consecutive lanes on consecutive
   addresses. Fixed by allocating, copying and synchronising through cudart.

2. The query norm was summed across warps that had each computed the same
   partial: the inner loop already covers all of D within one warp, so the norm
   came out sqrt(8) too large on a 256-thread block. Every unnormalized case sat
   at float32 rounding while the normalize cases were off by ~1e-2, which is what
   pointed at it. Warp 0 now computes it alone.

   kernel_simulation.py did NOT catch this, because it was written from the
   intent rather than transcribed from the .cu. A simulation is only as good as
   its fidelity; hardware is what settles it.

3. The manifest claimed sm_89 while build.sh defaults to sm_80. The contract
   checker from ThinkFlowLab#25 caught that, as it caught the same class of over-claim in the
   Laya backend.

The manifest is now status=validated with a declared tolerance and a reference
entrypoint, which is what the contract requires of a backend making a parity
claim. The reference is float64 in reference.py, and gpu_parity.py skips with a
reason rather than passing when there is no GPU or no library.

Not yet exercised: the batch path beyond questions=1, the K>255 rejection, and
any architecture or CUDA version other than sm_89 on 13.0.
xiaoyu-xyz added a commit to xiaoyu-xyz/system1-omni that referenced this pull request Sep 28, 2026
Both models that need this readout share the shape of it: Kev (ThinkFlowLab#27) scores
scale * dot(k_proj(c_i), q_proj(s)) and softmaxes over a question's candidates;
CLM (ThinkFlowLab#9) scores exp(logit_scale) * cos(state_head(s), action_head(c)) and does
the same. ThinkFlowLab#9 asks for exactly this fusion and ThinkFlowLab#19's gemm.cu does not have it.

The fusion is more than fewer launches. For a cosine the candidate's norm and its
dot with the query need the same elements, so both accumulate in one read of C --
for Kev, 255 rows of 2560 floats read once instead of twice. The softmax runs
in-block over shared memory, so there is no second kernel, no atomics and no
global round trip for the similarities. A batch is a grid over questions.

MEASURED, not just intended. RTX 4090 (sm_89), driver 595.58.03, CUDA 13.0
(V13.0.88):

    worst absolute difference: 1.835e-07 (tolerance 0.0001)
    all cases within tolerance

All 13 fixed cases pass, including a single candidate, identical candidates, a
zero candidate, K=255, and logits whose raw exponential overflows.

Three defects were found and fixed; the first two only a GPU could show.

1. The parity harness passed host pointers where device pointers were expected,
   so the kernel dereferenced host memory as device memory. compute-sanitizer
   located it as an invalid global read with consecutive lanes on consecutive
   addresses. Fixed by allocating, copying and synchronising through cudart.

2. The query norm was summed across warps that had each computed the same
   partial: the inner loop already covers all of D within one warp, so the norm
   came out sqrt(8) too large on a 256-thread block. Every unnormalized case sat
   at float32 rounding while the normalize cases were off by ~1e-2, which is what
   pointed at it. Warp 0 now computes it alone.

   kernel_simulation.py did NOT catch this, because it was written from the
   intent rather than transcribed from the .cu. A simulation is only as good as
   its fidelity; hardware is what settles it.

3. The manifest claimed sm_89 while build.sh defaults to sm_80. The contract
   checker from ThinkFlowLab#25 caught that, as it caught the same class of over-claim in the
   Laya backend.

The manifest is now status=validated with a declared tolerance and a reference
entrypoint, which is what the contract requires of a backend making a parity
claim. The reference is float64 in reference.py, and gpu_parity.py skips with a
reason rather than passing when there is no GPU or no library.

Not yet exercised: the batch path beyond questions=1, the K>255 rejection, and
any architecture or CUDA version other than sm_89 on 13.0.
Cs1GemmPlan now records the cuBLASLt version a plan was tuned with, and
import refuses a plan from another version, so the check no longer
depends on the caller (ABI version 2; the plans file format is
unchanged). A shape that was not tuned borrows a tuned algorithm only
from an M at most twice its own, or from the largest tuned M when
nothing larger was tuned, and otherwise takes cuBLASLt's first choice.
An exhaustive tune fails when cuBLASLt cannot list its algorithms
instead of timing only the shortlist.

Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
@twu3202

twu3202 commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, this is useful, especially with Kev planning to load the same library.

  1. In this worker every length in TUNE_ROWS is tuned, from 64 to 16384 and never more than 2x apart, and a Cua-S1 prompt is always well over 64 tokens, so the tuned M it borrows from is always within 2x. Only prompts past 16384, if the limit is raised, take the largest one below, and that is deliberate: on sm_89 the heuristic's first pick for long M is a half-rate kernel. Other callers won't have that guarantee, so plan_for now borrows from above only within 2x, from below only when nothing larger was tuned, and otherwise takes the heuristic's choice. Results for this worker are unchanged.
  2. The flush does survive compilation: cuobjdump -sass on the built library shows both loads in the loop (LDG.E [R2.64] and [R2.64+0xc]) and the conditional STG.E, since the compiler can't know what the buffer holds. Reading two words of each 16-byte element still pulls every 32-byte sector through L2, so I've left it as is.
  3. Agreed. Cs1GemmPlan now carries the cuBLASLt version and import refuses a plan from another version, with CS1_ABI_VERSION at 2. The worker already refused plans files from another cuBLASLt version, but the library shouldn't rely on its caller for that. There's a GPU test for it in tests/kernels.rs.

Also taken: an exhaustive tune now returns an error when cuBLASLt can't list its algorithms. log_choice reading the variable once is intended.

Pushed as 637ced1. With the existing plans file the scores are bitwise unchanged, and a fresh --gemm-search run passes the accuracy check.

Bring in the README change from the ThinkFlowLab#11 review: one src/models/ row in
the layout table instead of one row per model.

Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
Bring in the README change from the ThinkFlowLab#11 review: one src/models/ row in
the layout table instead of one row per model.

Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
Rust 1.98 adds clippy's chunks_exact_to_as_chunks, which fails the three
bfloat16 decodes under -D warnings; they now use as_chunks. The recipe and
test scripts pass ruff check (E4, E7, E9, F, I) and ruff format, as the
multimodal workflow runs them over recipe/cua_s1. The reformatted scripts
produce the same corpus, float vectors and printable table as before.

Signed-off-by: Tianyao Wu <rayroy31@gmail.com>

@hsliuustc0106 hsliuustc0106 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed commit 6104d17cc0d879c726002be64f6a8ac3e91e00e6.

No actionable findings in native-worker source review (request mapping, tokenizer export, loading, buffer layout, graph ownership, launch boundaries). Rust library tests: 9 passed, 1 ignored (external float vectors). CUDA kernels and model parity not executed; source review is not numerical certification.

@hsliuustc0106

Copy link
Copy Markdown
Contributor

cleanup it

Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
Move the HTTP worker to src/frontend/cua_s1_text.py, so src/models/cua_s1/text/
holds only the model: contract.py (request mapping, prompts, answers) and
model.py (adapter checks, loading, readout; formerly engine.py and adapter.py).
The upstream MIT notice moves into contract.py. The upstream comparison and
latency scripts are no longer part of the change; the recipe keeps setup and
launch only.

Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
Drop the comparison and benchmark tooling (diff_corpus.py, diff_workers.py,
check_native.py, and the worker's --score-all, --bench and --encode-only
modes with the eager/served switch they used), the float-vector generator,
and the Unicode table that made error messages escape non-ASCII names
exactly as Python's repr does, along with code only they used. The export
record and tokenizer are now checked once at startup, and the export script
refuses weights without download metadata, which the worker would refuse
later. The upstream MIT notice moves into contract.rs, and the README and
recipe keep setup, launch and tests.

Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
@twu3202

twu3202 commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, cleaned up in a086a31. It drops the comparison and benchmark tooling (diff_corpus.py, diff_workers.py, check_native.py, and the worker's --score-all, --bench and --encode-only modes), the float-vector generator, the Unicode table that made error messages escape names exactly like Python's repr, code only those used, and THIRD_PARTY_NOTICES.md (the notice is now in contract.rs). The diff on top of #13 goes from 8.6k to 6.8k lines, about 0.9k of them Cargo.lock. Statuses and answers are unchanged, and the removed tools are kept at 48ed0e7.

If it helps, once #34 and #35 land I can move serving into the frontend's in-process engine, which would take the HTTP code out of this crate.

Keep the model, its HTTP worker and the tests that exercise them. The
checked-in input set, revision detection, extra flags, bearer auth and
duplicated validation go; the limits become constants. The README now
describes the worker as the correctness reference for native execution.

Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
@hsliuustc0106

Copy link
Copy Markdown
Contributor

is this ready for review?

Signed-off-by: Tianyao Wu <rayroy31@gmail.com>

# Conflicts:
#	src/models/cua_s1/README.md
Run each prompt eagerly with cuBLASLt's first-choice GEMM algorithms; CUDA
Graphs and GEMM tuning move to a follow-up. Parse requests with serde_json
instead of emulating CPython's json module, serve from main.rs without the
extra configuration, and check the attention kernel against a float64
reference instead of a second kernel.

Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
@twu3202
twu3202 marked this pull request as ready for review September 30, 2026 10:06
@twu3202

twu3202 commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

Yes. I've just pushed the trimmed version: the minimal eager path, with CUDA Graphs and GEMM timing left for a follow-up PR, and the description now has its results.

@twu3202

twu3202 commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

For the follow-up: the previous version of this PR captured CUDA Graphs and timed cuBLASLt's candidate GEMM algorithms at startup, without the exhaustive search and plan files. Measured with the same bench on the same GPU, in a separate run, p50 in milliseconds:

Prompt tokens This PR With Graphs and GEMM timing
139 18.2 15.7
218 24.8 20.6
292 32.5 27.6
712 68.8 60.0
15,446 1588.5 1324.6

Startup goes from 1.9 s to about 6 s with the timing. I'll rewrite that part on top of this PR, test it again, and open it as a separate PR.

@hsliuustc0106

Copy link
Copy Markdown
Contributor

generally lgtm, please fix the minors

@hsliuustc0106

Copy link
Copy Markdown
Contributor

please help review PR #52 as well

@hsliuustc0106
hsliuustc0106 merged commit a6f7b23 into ThinkFlowLab:main Sep 30, 2026
1 check passed
cacheline999 added a commit to cacheline999/system1-omni that referenced this pull request Oct 1, 2026
- src/frontend/laya_mps.py: the HTTP worker, started with flags
  (PYTHONPATH=src python -m frontend.laya_mps --device mps --model english
  [--compile] [--weights fp16] [--require-device]) instead of LAYA_WORKER_*
  environment variables. laya-serve's own variables (LAYA_API_KEY, ...)
  still apply.
- src/models/laya/: model-side code only. engine.py holds the warmup,
  the per-model description /health returns and the revision record;
  optimize.py is unchanged.
- tests/laya/: the unit and contract tests, run with PYTHONPATH=src.
- recipe/laya/requirements-mps.txt pins the validated environment.
- recipe/laya/bench/: 10 scripts become 6. The answer comparison is a
  section of report.py, the frontend-overhead probe is paired.py's
  --a-url/--b-url mode, and the workload generator and token check are
  replaced by the committed workloads.jsonl plus its token counts in the
  README. The five result tables leave the repository and join the raw
  data as a release asset.
- README: LAYA's row in the models table points at the Apple Silicon
  worker.

No behaviour change: 26 unit and 13 contract tests pass, and a
feasibility pass of every script gives the same numbers as before
(68-token request 28 ms with both options, paired ratio 0.64, answers
within 0.0031, frontend overhead ratio 1.00-1.01).
cacheline999 added a commit to cacheline999/system1-omni that referenced this pull request Oct 3, 2026
- src/frontend/laya_mps.py: the HTTP worker, started with flags
  (PYTHONPATH=src python -m frontend.laya_mps --device mps --model english
  [--compile] [--weights fp16] [--require-device]) instead of LAYA_WORKER_*
  environment variables. laya-serve's own variables (LAYA_API_KEY, ...)
  still apply.
- src/models/laya/: model-side code only. engine.py holds the warmup,
  the per-model description /health returns and the revision record;
  optimize.py is unchanged.
- tests/laya/: the unit and contract tests, run with PYTHONPATH=src.
- recipe/laya/requirements-mps.txt pins the validated environment.
- recipe/laya/bench/: 10 scripts become 6. The answer comparison is a
  section of report.py, the frontend-overhead probe is paired.py's
  --a-url/--b-url mode, and the workload generator and token check are
  replaced by the committed workloads.jsonl plus its token counts in the
  README. The five result tables leave the repository and join the raw
  data as a release asset.
- README: LAYA's row in the models table points at the Apple Silicon
  worker.

No behaviour change: 26 unit and 13 contract tests pass, and a
feasibility pass of every script gives the same numbers as before
(68-token request 28 ms with both options, paired ratio 0.64, answers
within 0.0031, frontend overhead ratio 1.00-1.01).
xiaoyu-xyz added a commit to xiaoyu-xyz/system1-omni that referenced this pull request Oct 3, 2026
All three come from review on ThinkFlowLab#25.

1. reference.entrypoint was used as a path without checking its type. A list
   reached os.path.isabs, whose TypeError escaped check_manifest: the CLI printed
   no structured report at all, not even under --json, and every later backend
   went unchecked. Non-strings are now an Issue and checking continues.

2. A directory holding any *.backend.json was exempt from the undeclared-backend
   check, but discovery only loads <dir>/<dir>.backend.json. A manifest named
   typo.backend.json therefore bypassed every check for that backend: exit 0,
   checked: [], no errors and no warnings. A manifest that is present but not
   under the expected name is now reported.

3. Architectures read from `arch=` variables and targets written literally in a
   -gencode flag were combined with `or`, so the literals were discarded whenever
   a variable existed. A script assigning `arch=${2:-89}` and then compiling
   `-gencode arch=compute_90,code=sm_90` builds sm_90 only, yet a manifest
   declaring [89] passed with no findings.

   build_script.py now reports gencode_architectures: the targets that actually
   reach nvcc, resolved against the variable environment in force at each line,
   so a loop that reassigns `arch` still yields both targets. The checker treats
   those as the authority and reports an assigned value that never reaches a
   -gencode flag.

62 tests, seven of them new and one per defect. Verified against ThinkFlowLab#19's real
build.sh, which still parses to variables [89] / gencode [89] and passes with no
findings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants