Repository navigation
Conversation
Signed-off-by: psv666 <2693925048@qq.com>
Signed-off-by: psv666 <2693925048@qq.com>
Signed-off-by: psv666 <2693925048@qq.com>
Signed-off-by: psv666 <2693925048@qq.com>
Signed-off-by: psv666 <2693925048@qq.com>
Signed-off-by: psv666 <2693925048@qq.com>
Signed-off-by: psv666 <2693925048@qq.com>
Signed-off-by: psv666 <2693925048@qq.com>
Signed-off-by: psv666 <2693925048@qq.com>
Signed-off-by: psv666 <2693925048@qq.com>
Signed-off-by: psv666 <2693925048@qq.com>
Signed-off-by: psv666 <2693925048@qq.com>
Signed-off-by: psv666 <2693925048@qq.com>
hsliuustc0106
left a comment
There was a problem hiding this comment.
Draft PR review: #197 "Kev: add eager native Rust/CUDA text worker with BF16 service parity"
Reviewer pass: first review, pass102 (2026-10-31). Verdict: COMMENTED (draft PR — no approval or readiness implied). ## Status block
- Base:
main@22b7717c9(PR-declared and merge-base-confirmed). Head:b7c94b34a60f35a5b88438259c94dca8089d08a4(matchesorigin/pr/197and PR APIheadRefOid; rechecked this pass). - Diff (two-dot
git diff 22b7717c9..b7c94b34a, identical to the GitHub API numbers): 72 files, +6,980/−87 (total changed lines 7,067, matching the body). The briefing's "87 files +7016/−784" does not match the current PR state at this head; API and local git agree on 72/+6,980/−87. - Draft state: DRAFT (API
isDraft: true); the body explicitly keeps human checkboxes unchecked pending human review — correct for a draft; not filled by this review. - Standing sync note: merge-base
22b7717c9is an older main; current tipe573260cc.git merge-treeshows no content conflicts against the current tip, but a main sync before ready is required (CI revalidation at the new base). - Author: psv666; head hosted on the psv666 fork (
codex/issue-27-kev-native).
Skill / evidence provenance
- Skill:
.agents/skills/system1-omni-review/SKILL.md, bytes read this pass, SHA-25658be3bc60806c4c5af7038a0c6a80fadb94f3360d714556a3e965c8903dde108(commitef9279638). References read:docs/architecture.md,CONTRIBUTING.md,references/repository-contracts.md. - Separation of sources: [inspected] = diff/tree read at pinned SHAs; [run] = commands executed locally; [CI] = Actions API at the exact head; [author] = body claims not independently verified.
Relationship to #170 (checked as requested)
- #170 "Kev: add pinned CPU export and FP32 pointer-head validation" is still OPEN and DRAFT, head
8f765fdbf9eb782ab6f9d731df359699b70398ba. [CI/API] - That SHA is an ancestor of #197's head (
git merge-base --is-ancestor→ yes), and the body pins exactly that SHA as the dependency.git log 8f765fdb..b7c94b34shows the 10 worker commits on top. The dependency pin is currently accurate. - F-1 (P1, merge-order dependency): #197 embeds the entire #170 slice; #170 must merge first. Until then, #197's Files-changed double-counts the export slice, and any rebase of draft #170 invalidates the pinned SHA and requires a #197 rebase. The body declares this ("Preserve #170 first in the merge order") — recorded as a standing constraint, not a defect. Trigger: merging #197 first or #170 moving. Consequence: duplicated review surface; stale pin.
- The export manifest contract is shared and consistent:
recipe/kev/export.py(from #170) was extended after8f765fdb(+22 lines:merge_numerics {device, accumulation, torch}written intokev_export.json; CUDA-merge default withtorch==2.8.0+cu128, TF32 off, FP32-matmul-highest;--merge-device cpukept for diagnostics), and the worker'scheckpoint.rspins every manifest field viarecipe/kev/pins.json(merge_numerics.device == "cuda"). Consequence: CPU-merged exports from the original #170 form are rejected with an explicit message — the body's "CPU diagnostic exports must be regenerated with CUDA merging" is enforced in code, not just documented. [inspected]
Template / body check
- Four post-#138 contributor answers: present, explicitly reused from the contributor's #170 answers (2026-10-25) with attribution, plus "Is a PR needed?" and observed failure evidence (the 139/3,393-question BF16 gate failures with max errors, before the fixes). [inspected]
- Checklist: present, four boxes unchecked with the statement "Human checkboxes are intentionally unchecked" — appropriate for a draft; the author's own self-review statement is separate above it.
- Headings follow the template (Purpose / Test Plan / System1-Omni Version / Commit: / Test Result / Demo / evidence with an explicit N/A reason for performance charts / Self-review).
- Large-code-change disclosures present: authored vs total counts (4,654 authored / 7,067 total, 72 files full; worker-only slice 3,920 authored / 5,367 total, 63 files), component/integration review statement, two-slice split rationale, per-area validation. My recomputation of the worker slice (
76b764ae..b7c94b34) gives 64 files, +5,269/−139 — approximately consistent with the body's 63-file claim; the small delta is range-definition ambiguity, not a discrepancy flag.
Commit-by-commit review [inspected]
4be35df4(#170 slice) pinned CPU export + FP32 pointer head (+1,764).8f765fdb(#170 tip) validate every pinned LoRA projection target (+39/−2).76b764aemerge commit "Merge current main into Kev export and head preparation" — parents4be35df4+22b7717c9. Hygiene: parent 2 is exactly the PR merge-base, so the merge is a clean bring-up to the review base; diff vs parent 2 contains only the branch's own 20 files; no evil-merge content found.0102113feager native Rust CUDA text worker + parity harness (28 files, +1,995).57e239b6reject invalid parity evidence and report boundary flips (gate-integrity tests).1d44e4f5docs: eager numerical and serving acceptance limits.490d80d3fix: reference model lifetime for lint (+3/−1).33e1848equalify against default BF16 service + Unicode tokenization fix (13 files, +990).b446d6eedocs: mark BF16 qualification as failing — the honest failure record before the fix; history preserves the failure arc rather than rewriting it.224fa3a8test: verify rendered prompts across the frozen service corpus.0efa1d9ffix: align eager native numerics with the pinned BF16 service (27 files, +1,653 — the Torch28 operator variants).d850153atest: qualify Noul boundaries using official service outputs.b7c94b34docs: record BF16 acceptance and shared Qwen regression evidence.
Linear story: build → discover mismatch → mark failing → fix (Unicode, numerics) → requalify → document. Good hygiene.
Contract review (Valen-native bar)
Export-as-load-contract, hash-enforced — yes [inspected src/models/kev/native/src/checkpoint.rs]
recipe/kev/pins.jsonis compiled into the binary (include_str!) and every manifest field must equal the pin (unsupported Kev export fieldotherwise).- Exact file-set equality (7 files, no extras), per-file SHA-256 recomputed from disk, pinned base
config.json/tokenizer.json/tokenizer_config.jsonhashes cross-checked, pinned FP32 head hash hard-coded, head tensor shapes checked (q/k[256, 2560]= Kev-4B). - Shadow protection: rejects an unlisted
model.safetensorsthat would shadow the verified shards; weight-map entries must bemodel.language_model.*in the two pinned shards. - Pins match the body's declared identity: revision
139fdd94, base1001bb4d, reference2074cc44, torch2.8.0+cu128, BF16 backbone / FP32 head. All verified againstpins.jsonandexport.py.
Wire rejection corpus vs the reference [inspected + run + CI]
- Committed fixtures:
tests/kev/data/unicode-processing.json(Unicode prompt/token/readout corpus),boundary-sources.json(33 frozen boundary question sources selected by actual official unrounded probabilities),head.json,planning.json,processing.json— maintained fixtures with generators, not one-off output. Raw 5,662-question captures stay outside Git (declared). - Gate mechanics tests (
tests/kev/test_service_acceptance.py,test_validate.py) enforce that failures cannot be rounded away: score can fail while all probabilities pass; raw-vs-wire score gates independent; gate changes invalidate discovery; missing/duplicate/reordered cases rejected; non-finite values rejected. Boundary flips are always reported; near-boundary coverage ≥10 per kind with far-boundary margins declared. - CI-wired portion: new
kev-exportCI job (CPU torch 2.8.0, no GPU/weights) runspython -m unittest discover -s tests/kev -p 'test_*.py'; CUDA contract checks and CUDA compilation cover the backend textually/compile-only. The full corpus replay is a GPU-local, documented procedure (recipe/kev/service-acceptance.md,noul-boundary.md), not CI-executable — correct scoping, but the recorded-corpus byte-parity at any future head remains an author-run check. [CI + author]
Finisher byte-parity (CPython float repr) [inspected src/models/kev/native/src/postprocess.rs]
rounded()implements Pythonround(float(x), 4)with ties-to-even viaformat!("{value:.4}")parsing (Rust formats the exact f64 decimal and rounds half-to-even — matching CPython's_Py_dg_dtoabehavior for this use), then serde_json shortest-roundtrip serialization matchesreprfor the rounded values.billing_json()reproducesjson.dumps(..., ensure_ascii=True)including\uXXXXescapes and surrogate pairs, withDEL(0x7f) correctly escaped; answer objects are built in the official insertion order ("billing tokenization observes it").- Calibration: pinned temperature
2.406050072164233(from pins), FP32 softmax, finite/range/sum validations, typed Noul/Choice/Score answer construction with mode/confidence formulas mirroring the official interpretation. Corpus/gate tests exercise these paths weight-free.
BF16 rounding locations [inspected CUDA]
service_readout.cu(new, compile-guarded byCS1_KEV_SDPA): CUTLASSmem_eff_attentionBF16 kernel instantiation matching PyTorch 2.8 SDPA (BF16 Q/K/V/mask, FP32output_accum), additive mask in BF16 with the documented visibility rule; other builds do not compile it, so no cross-profile drift.norm.cu:gated_rms_norm_kerneltemplatedTorch28— replicates Torch 2.8 reduction order (ascending shufflestep=1..16vs legacywarp_sum) and element mapping (lane + 32*jvslane*PER), with identical BF16 rounding points (round_bf16(v*inv),round_bf16(w*normed),to_bf16(scaled*silu)); the legacy export instantiates<false>— old arithmetic unchanged.gdn_fp32.cu:Torch28template onprepare(element mapping + shuffle order), FP32 state,cs1_gdn_prefill_fp32_xwith optional final-state output and zero-state null; context lifetime documented to outlive stream drain.- ABI: all new symbols additive (
ops.h"Optional Kev Torch 2.8 default-service profile"); Rustcuda.rsresolves them with?— hard-required for the Kev profile (.context("Kev requires recipe/kev/build_cuda.sh")), optional for everyone else.
No silent fallbacks / readiness / ownership [inspected]
Model::enable_service_readout_numericshard-fails on missing symbols, wrong 4B geometry, or any graph/lora/reference/batched/packed state, before first forward. Eager is explicit (load_with_graph(dir, lib, false)). No fallback path exists.main.rs: load →engine.warmup()(real mixed Choice/Noul/Score inference, error aborts startup) → then bind/listen. Readiness-after-real-init satisfied.- Serial admission (
SerialScheduler, executor mutex, poison → error), permit retained until all submitted work drains including row failure/disconnect (executor.rssync-on-both-paths), HTTP content-type 415, admission 503 + Retry-After, cancel-on-drop. Matches the runtime contract and frontend requirements at this scope (own binary; shared-frontend integration claimed viamodel/service.rs+ 55 worker/frontend/native comparisons [author-reported]).
Cross-PR hygiene [run]
- F-2 (P2, coordination): #197 genuinely conflicts with sibling #196 —
git merge-tree origin/pr/196 origin/pr/197reports content conflicts indocs/architecture.mdandsrc/models/qwen3_5/native/src/model.rs(both extend shared Qwen execution from the same base; both also touchnorm.cu/ops.h/cuda.rs, which auto-merge but are semantically adjacent). Whoever merges second must do a real conflict resolution in the shared executor file; plan the order (#170 → then coordinate #196/#197) and re-run shared numerical checks after resolution. - #197 × #198: clean. Both × current main tip: clean (0 conflict markers).
CI (observed) [CI]
Actions API filtered to exact head b7c94b34a..., event pull_request:
- ThinkFlowLab/system1-omni: CI run 40649603220 success (the run ID the body cites); Docs run 40649603196 success.
- psv666/system1-omni (fork): no workflow runs recorded at this SHA (empty result). The body's CI link points at the upstream/mirror run; recorded as observed.
Locally executed [run]
Environment: macOS arm64, no cargo/GPU; Python 3.12.8, pytest 8.4.1, mkdocs 1.6.1; local conda torch is broken (missing libtorch_cpu.dylib). Worktree at b7c94b34a, removed after.
PYTHONPATH=. python -m pytest tests/benchmarks -q→ 133 passed.python -m unittest discover -s tests/kev -p 'test_*.py'→ 16 tests: 15 passed, 1 error — the single error istest_exportfailing at import because the local torch install is broken (environment issue, not PR code). In CI (green at this head) torch==2.8.0 CPU is installed and the same command passes.mkdocs build --strict→ exit 0.- Rust checks and all GPU/head comparisons: not executable locally; CI covers compile + weight-free Python; GPU acceptance numbers are author-reported.
Material unverified checks
- The 5,484-request / 5,662-question BF16 acceptance campaign, the 99 bit-exact operator comparisons, the byte-for-byte 52-request boundary/contract capture, and the 2,269-question fresh Noul replay are author-reported (raw evidence is local to the author:
evidence/precheck-v5/,/tmp/system1-issue-27/handoff.md). - CUDA compilation CI is not GPU inference validation (the body says so itself); sm_89 build vs "Torch reports 9.0" discrepancy is disclosed but unresolved hardware nuance.
- Shared
model/service.rschanges to the default Qwen consumers: code inspection shows additive/optional symbols only; existing Qwen regression suites pass per author + CI, but no independent GPU recheck was possible in this pass. - #170 may move; re-pin and rebase before merge (F-1).
…port-review Signed-off-by: psv666 <2693925048@qq.com>
Signed-off-by: psv666 <2693925048@qq.com>
Signed-off-by: psv666 <2693925048@qq.com>
Signed-off-by: psv666 <2693925048@qq.com>
|
In response to this review. Thanks for the review. Follow-up head:
The NumPy dependency fix and synthetic full-export tests are reviewed independently in #170 and inherited here; the worker tests explicitly request CPU merging, preserving the CUDA serving-export contract. New-head |
|
Verified at 9501ede. The worker follows the pinned reference numerics closely, the optional Qwen entry points check out at every call site, the scheduler and executor lifetimes are sound, and CI is green at this head. Two small findings. P3 in recipe/kev/reference-requirements.txt:3 the bare torch==2.8.0 pin resolves from the default PyPI index while the qualification environment is torch 2.8.0+cu128, so a fresh reference install can silently drift from the CUDA build the native side compares against, state the cu128 index requirement in the file or pin the local version. P3 in src/models/kev/native/src/postprocess.rs:27 the rounded helper formats a string and calls expect on the serving path, so a future formula change that yields NaN or inf turns a formatting failure into a panic instead of a controlled error, returning a Result and propagating it as a 4xx or 500 would be safer. |
Signed-off-by: psv666 <2693925048@qq.com>
|
@MrDongsls Thanks for checking the current worker. I verified the two follow-ups separately.
This commit only changes the reference requirements and installation documentation. The existing missing historical raw-evidence and #196 integration limitations remain; the PR remains draft. Current-head |
Purpose
Add Kev-4B's first eager native Rust/CUDA text worker for Choice, Noul and Score, aligned with the pinned official BF16 default service. This completes the implementation scope of #27; human review remains pending.
Depends on the independently reviewable export/FP32-head slice in #170 at
85119d093fa3cb6ae5244b1d5c7eb2619084d9a2. Review only the worker follow-up. Both PRs target upstream main; until #170 merges, this PR's Files changed also contains that prerequisite. Preserve #170 first in the merge order.Summary
The worker validates a pinned CUDA-merged export, performs real mixed-question warmup before listening, and supports the shared frontend. Model-local planning matches official state/branch padding and projection shapes. Optional Qwen/CUDA operators match Torch 2.8 reductions, BF16 rotary buffers, FP32 GDN state and masked SDPA. Temporary state belongs to one question; no cross-request prefix reuse is added.
Optional CUDA builds use pinned PyTorch/CUTLASS headers and ship their license notices. Serving has no Python/libtorch runtime dependency. Existing Qwen default/reference callers retain their dispatch and ABI. CPU diagnostic exports must be regenerated with CUDA merging for this worker's serving contract.
Review follow-up (2026-10-31)
Current head
9501edeae0d9f293dc954b9de09def57af2ae173contains main5fa5179d57f9267bb0d20162004d8ce26352aa9dand updated #17085119d093fa3cb6ae5244b1d5c7eb2619084d9a2as ancestors; no force-push or rewritten original commits. The worker-only comparison starts at that updated export/head SHA, not the earlier main-merge commit.prefix_layout/prefix_byteswhile retaining the public profile guards and private per-question allocation. The two newcapture_fixed/continue_fixedcallers explicitly pass no Kev service window. A pre-fix compile failure recorded both missing arguments; the integrated build now passes.model/service.rsarithmetic changed fromb7c94b3.gpu run; the one checkpoint-dependent kernel test is explicitly filtered out. The separately enabled pinned-head oracle passes its original tolerances (logit3.12924385e-7, probability2.98023224e-8).9152ee46b2f8a2589d64f8f193919d8916b96618still conflicts indocs/architecture.mdandsrc/models/qwen3_5/native/src/model.rs. Kev: add pinned CPU export and FP32 pointer-head validation #170 must merge first; whichever of perf(open-jev): optimize 9B prefill while preserving native outputs #196/Kev: add eager native Rust/CUDA text worker with BF16 service parity #197 merges second must resolve the shared executor and rerun shared numerical checks. This constraint is not marked resolved and unrelated perf(open-jev): optimize 9B prefill while preserving native outputs #196 code is not folded into this PR.Both PRs remain draft. New-head
9501edeCI and Docs now pass: Rust, benchmarks, Kev exporter, CUDA contract and CUDA compilation are green; docs deploy is skipped as configured. CUDA compilation is not full-model GPU qualification. Prior CI results below retain their original SHA.Required human contributor answers
Reused from the contributor's explicit answers in #170, supplied on 2026-10-25 (translated):
Is a PR needed? The contributor explicitly requested implementation PRs where appropriate and now requested the final self-review and push. The export/head and complete-worker slices remain separately reviewable.
Observed failure evidence: The earlier BF16 run had 139/3,393 questions above the probability gate (maximum 0.06492195), Score-255 error 0.16675543 and three near-boundary decision flips. The committed diagnosis records causes and fixes; original failures, layer traces and intermediate unsuccessful probes were retained in the original checksummed handoff; that raw archive is currently unavailable after the temporary-directory loss described below. No gates were relaxed and no failing sample was removed.
Test Plan
System1-Omni Version / Commit: fetched main
5fa5179d57f9267bb0d20162004d8ce26352aa9d; export/head85119d093fa3cb6ae5244b1d5c7eb2619084d9a2; complete worker9501edeae0d9f293dc954b9de09def57af2ae173. Current diff: 4,828 authored-code-file / 7,261 total changed lines, 73 files; worker-only diff from the updated #170 head: 3,965 / 5,423, 65 files. Authored-code counts include comments in.rs/.py/.cu/.cuh/.c/.h/.cpp/.sh, excluding data/config/docs/licenses. This received a full component/integration review. The requested two-slice boundary is preserved; processor geometry, shared numerical execution and calibration must work together to qualify the complete worker. Numerical repair, boundary tests and documentation are separate follow-up commits.Official
2074cc44256200bb37540103fa4572901faa10f3; base1001bb4d826a52d1f399e183466143f4da7b741b; adapter/head139fdd94f1b6a6ad80cc15e08fcb99cac885a101; temperature2.406050072164233. Torch 2.8.0+cu128, Transformers 5.17.0, PEFT 0.21.0; BF16 backbone, FP32 head, merged LoRA, default SDPA/Graph/cache enabled, optional FLA/causal-conv1d absent. FP32 controls are diagnostics, not the primary acceptance target.Linux x86_64, lab L20X-labeled GPU, one GPU through
gpu run, sequential requests with fixed ordering and mixed questions. CUDA build sm_89 plus compute_89 PTX; Torch reports capability 9.0 while nvidia-smi reports 8.9, so there is no separate sm_90 qualification claim.Main commands:
Build and serving recipe, acceptance protocol, boundary method and shared regressions. Exact GPU commands, library hashes and raw logs are in the author-local
evidence/precheck-v5/; full reproduction isgpu run --gpus 1 --wait 2h --timeout 2h --note 'Kev BF16 reproduction' -- bash /tmp/system1-issue-27/reproduce-bf16.sh /absolute/fresh/outputon the recorded host. That script was syntax-checked; its component campaigns were executed, not an additional monolithic whole-corpus rerun.Test Result / Evidence
The 2,269-question new Noul pool is frozen and deduplicated before inference; actual official unrounded probabilities select 33 new boundaries before native runs. Every candidate is retained and compared. All 36 Noul boundaries qualify again in a fresh official/native replay with zero within-implementation probability drift. Original worst probability/Score cases match embeddings, all 32 residual layers and final norm. FP32 controls remain separate, including native versus historical FP32 Score-255 difference 0.28167720.
Historical v5 precheck at
b7c94b3(not rerun in full at the updated head):cs1_*exports remain. No new mandatory Python/libtorch runtime dependencies.The full 5,662-question result is the retained v4 campaign. Final precheck changes packaging, coverage and documentation, with functional model/kernel files hash-unchanged; it freshly reruns the shared-engine and boundary/HTTP checks above. Historical v1-v4 failures and captures are preserved, not relabeled as fresh runs. Committed fixtures have active tests/generators; raw logs, captures, tensors and weights stay outside Git.
Previous-head
b7c94b3CI passes Rust, benchmarks, Kev exporter tests, CUDA contract checks and CUDA compilation; the current Docs build also passes (deploy skipped as configured). CUDA compilation is not GPU inference validation.Demo / evidence
Before: no qualified native Kev text worker. After: Choice/Noul/Score requests are served through native and frontend paths with the frozen checks above. The linked recipe and numerical/boundary reports contain reproduction instructions and actual measured results. The original checksummed evidence and PR text were delivered under
/tmp/system1-issue-27, but those files are now unavailable. The restored handoff records current review fixes/checks and the missing-evidence boundary; it does not recreate the historical raw captures. No latency/throughput/memory comparison was performed, so performance charts/video are N/A.Merge Danger
Door: two-way. Additive serving support; existing CPU exports need regeneration for native serving.
Blast Radius: Qwen. New numerical entry points are optional and selected by Kev; old symbols, default dispatch, prefix/Graph checks and legacy arithmetic pass. Actual Cua/Open-Jev/JEMM task accuracy, other GPU architectures/sizes, optional FLA, arbitrary official concurrent batching, long-input generality and performance remain unverified. Remote CI is separate from the reported local results. No universal model-quality claim.
Self-review
Full automated/agent-assisted component review is complete. Contributor/maintainer review remains pending; this PR stays draft. Human checkboxes are intentionally unchecked.