Repository navigation
test(pipeline): Inc3c real-model capture validation + general Bool value clone - #541
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #541 +/- ##
==========================================
+ Coverage 80.59% 81.31% +0.72%
==========================================
Files 315 315
Lines 123259 123446 +187
Branches 123259 123446 +187
==========================================
+ Hits 99335 100382 +1047
+ Misses 19891 19009 -882
- Partials 4033 4055 +22
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
3860e1b to
6f48362
Compare
…ne proof + perf record) Standalone follow-up to #533 (Inc3c capture-step-inputs, merged). Validates the captured step-inputs decode path against the real multi-component inputs_embeds decoder class and banks the honest real-model finding + measured tok/s. Does not touch #533's reviewed code (native_decode/*). Refs #384. MEASURED (device 4, release, ORT 1.27.0 CUDA, two-length steady-state): qwen3-0.6b native-CUDA captured = 614.3 tok/s (1.42x ORT) qwen3-0.6b native-CUDA eager = 206.5 tok/s (0.48x ORT, loses ~2x) qwen3-0.6b ORT-CUDA reference = 433.0 tok/s ENGAGEMENT (counter proof on the real qwen3-0.6b): - qwen3-0.6b is single-component (input_ids): from_pipeline_dir refuses it ('metadata has no pipeline section'), and native-CUDA single-graph decode with ONNX_GENAI_NATIVE_DECODER_CAPTURE_STEP_INPUTS=1 leaves the counter at 0 - it DECLINES the flag (captures via the token-id run_one_token lever instead). So qwen3-0.6b is the wrong model class for this flag; the 614/206 table is its single-graph CUDA-graph capture lever (a faithful launch-overhead proxy). - The real inputs_embeds decoders (qwen3.5-0.8b hybrid, gemma-3n-e2b) are the correct class. gemma-3n's decoder is GroupQueryAttention capacity-aware KV so it WOULD engage capture; it is blocked by (1) a Bool audio-mask clone gap and (2) required vision inputs for text-only decode. The Bool audio-mask clone fix is delivered by the canonical #540 (clone_value + clone_owned, host-guarded, all POD dtypes), NOT carried here, to avoid a conflicting duplicate in decode/values.rs. Tests: - qwen3_0_6b_capture_step_inputs_decline.rs: concrete counter=0 decline proof. - gemma3n_native_cuda_capture_realmodel.rs: forward-looking real-model capture-engagement + token-parity harness; skips gracefully on the remaining vision-required-input gap (matches the qwen35_0_8b_hybrid skip precedent); relies on #540 for the Bool audio-mask clone. Verify: fmt --check clean; clippy x4 clean; full cuda,native-backend suite failing set 17, byte-identical to base, 0 regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
6f48362 to
10e6afa
Compare
|
VERDICT: APPROVE Independent review by Lori (reviewer; author Mary locked out of revising per reviewer-protocol). Reviewed on device 5, ORT 1.27, real models present. Branch is correctly based on #540 (merge-base Headline: the qwen3-0.6b decline test is GENUINELY non-vacuous — proven by running it
So it concretely proves the flag DECLINES on the real named model, not tautological green. Scope — exactly as specified, ZERO src changesAgainst the true merge-base #540 ( B. gemma3n skip-gate is NARROW-ENOUGH and matches merged precedent — RAN it
C. Decision note is accurate and consistent with the testsThe note's decline claim (from_pipeline_dir refused "no pipeline section" + counter=0 with flag ON) matches the test output exactly. OFF/ON/ORT table (614.3/206.5/433.0, captured beats ORT 1.42×) is internally consistent (614.3/433.0=1.42). Minor wording nit only: line 130 says "1.38×" while the table/headline say 1.42× — cosmetic, no contradiction with any test assertion. D. Regressions
Genuine non-vacuous decline proof, correctly scoped, honest note, no src risk. Approving. — Lori |
🔴 Benchmark Regression DetectedComparison of criterion micro-benchmarks: PR head vs merge-base, measured on the same runner in the same job (base first → PR second).
Visual flags: Host infoWhat this cannot catch
|
Scribe round 7 bookkeeping (docs-only, no code): - Merged **30** decision-inbox notes into `.squad/decisions.md` (20389 → 20458 B, under the 20480 gate); round-7 per-PR narrative archived verbatim to `decisions-archive/2026-07.md`. - Logged the #535 / #540 / #541 / #543 wave into mary/harry/cohaagen/melina history. - Cleared the processed inbox (README kept). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…#565) ## Wire pure-native multi-component pipeline decode (GAP-3 Inc-A) First slice of the Justin-approved native multi-component pipeline workstream (GAP-3). Previously `pipeline/mod.rs build()` rejected pure-native pipeline selection with *"native pipeline decode is not yet implemented"* — blocking native decode for any multi-component (split embed→decoder) model. ### What Removed the construction-time bail. The pure-native path (`PipelineBackend::Native`) now falls through to the **already-working, backend-agnostic** flat-autoregressive decode loop (`run_autoregressive` → `PipelineDecodeLoopBackend`), building every component as a native `ComponentSession` + the decoder as `NativePipelineDecoder` — the same thing the hybrid env-flag path builds, but driven by backend selection instead of env injection. - **DRY:** new `native_component_selection()` converges both native sources (Native-backend and Ort+env-flag hybrid) on ONE decision point + the SAME builders. No forked construction; hybrid (#543/#541) byte-unchanged. - **Scope:** NON-PAGED only — `use_native_decoder ⇒ paged_enabled=false`, so S2 `mirror_last_present_kv` (`decoder_component.rs:256`) is never reached (that's Inc-C). No decode-loop / `native_decode/*` / capture-core changes — zero overlap with the concurrent Scan work. - Non-flat-AR native plans get a precise `native_pipeline_plan_unsupported` error (no silent mis-route). Auto→Native is normalized so Auto-resolved-native also drives native selection. ### Evidence - **Token-exact 3-way** (CPU, `tiny-gemma4-vlm`): ORT oracle == hybrid-env == pure-native, all `[0,5,6,7]`. - **CUDA differential** (`tiny-gqa-embeds-cuda`): pure-native == hybrid, both `[0,5,6,7]`. - **Non-vacuous:** reviewer re-inserted the bail → test FAILS at the pure-native case. - Regressions green: #384 native parity, #543/#541 hybrid, #554 session-reuse, 343 lib tests. fmt+clippy clean. ### Review Independent review by Harry (author Cohaagen locked out on rejection) — **APPROVE** with mutation + regression evidence. ### Handoff - Inc-B: native `prompt_only` prologue (e.g. vision_encoder). - Inc-C: native present-KV mirroring / paging at `decoder_component.rs:256` (unblocks Qwen3.6-35B-A3B MoE). - Follow-up: native-only loader (pure-native still loads ORT `PipelineModels`; ORT-unloadable block-quant models need it). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…GAP-3 Inc-C) (#566) ## GAP-3 Inc-C — native present-KV mirroring → paged native pipeline decode Closes the S2 bail in `NativePipelineDecoder::mirror_last_present_kv` so the pure-native pipeline decoder (Inc-A #565) can run **paged** (cross-request KV prefix reuse), not only the non-paged flat-AR path. Host-KV path; device-resident present-KV (35B-A3B GPU) is explicitly deferred to Inc-D. ### What's wired - `native_decode/mod.rs`: `supports_host_kv_mirror` gate, `host_present_kv` (read), `seed_growable_kv` (seed). - `pipeline/decoder_component.rs`: real `mirror_last_present_kv` — byte-identical geometry to ORT's `mirror_present_kv_to_pages` (shared `extract_present_token`/`append_token_kv`); `supports_paged_kv` + `load_paged_prefix` seams. - `pipeline/flat_autoregressive.rs`: DRY-factored `claim_paged_prefix` shared by ORT + native admit paths (only the KV sink differs). - `pipeline/mod.rs`: read-only `#[cfg(feature="native-backend")]` test accessor `materialize_published_prefix_kv` (non-mutating prefix-cache read-back). - `tests/native_pipeline_backend_selection_parity.rs`: `native_paged_prefix_reuse_matches_fresh_and_ort` — warm==cold==ORT tokens **plus** direct native-vs-ORT paged-KV **byte-equality** assertion. ### Correctness - Token-exact 3-way: pure-native warm == native cold == ORT oracle; `reused=4`. - **Geometry non-vacuity (independently re-run by reviewer):** no-op mirror, key/value swap, zeroed mirror, and forced-reused=0 mutations ALL fail — the byte-equality assert catches geometry corruption that argmax alone missed. - No production regressions: Inc-A #565, #541/#543 hybrid, #554 session-reuse green. - Scope: zero edits to the parked capture core (`plan_capture_region`, `executor/capture.rs`, `CudaGraphLifecycle`). ### Deferred to Inc-D (35B-A3B GPU end-to-end) Device-resident present-KV read-out, in-place-GQA CPU KV, f16/non-rank-4 round-trip, MoE routed-expert specifics if needed. These decoders correctly gate `supports_paged_kv=false` → Inc-A non-paged fallback (no silent-wrong paged run). ### Reviews Independent opus review with strict author-lockout: Mary REJECTED the original (vacuous geometry test) → revised the test as authorized non-author → Harry independent re-review **APPROVE** (all mutations re-run, byte-equality confirmed, production byte-identical, deferral gate verified). Note: run the parity test binary with `--test-threads=1` (two tests share a process-global decoder-device env var; parallel races flake — pre-existing on HEAD). --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…DA decode (GAP-3 Inc-D) (#567) ## GAP-3 Inc-D — device-resident present-KV read-out → paged native CUDA decode Lifts the Inc-C (`#566`) `supports_paged_kv=false` gate for **device-resident f32 rank-4 CUDA GQA present-KV**, so a native CUDA pipeline decoder now runs **paged** (cross-request KV reuse) instead of the Inc-A non-paged fallback. Closes the present-KV threading gap for Qwen3.6-35B-A3B GPU decode. ### How (pure post-step plumbing — no kernel/capture-core edits) - `native_decode/cuda.rs`: `read_present_kv` reads the KV binding **after** the decode step's existing stream sync (`read_bytes`→`copy_to_host`→`dtoh`, which synchronizes) using the **physical/capacity** shape `[1,H,max_len,Dh]` so strides address the padded buffer; `seed_prefix` is the device counterpart of Inc-C's host seed; `device_present_kv_view` isolates the physical-shape handling. - `native_decode/mod.rs`: `present_kv`/`seed_kv`/`supports_device_kv_mirror` unify host-growable (Inc-C) and device-CUDA (Inc-D) onto the **same** `extract_present_token`/`append_token_kv` geometry + host f32 paged store (DRY, byte-comparable with ORT). - `pipeline/decoder_component.rs`: `supports_paged_kv = host OR device`; `mirror_last_present_kv &self→&mut self` (rippled to trait + ORT impl); `load_paged_prefix → seed_kv`. ### Correctness - `native_paged_prefix_reuse_matches_ort_on_cuda_device`: paged-native-CUDA == non-paged-native-CUDA == ORT oracle == closed-form tokens; mirrored pages **byte-equal** CUDA-vs-ORT; `reused=4`. - **H=2 unit test** for the physical-vs-logical stride bug (all existing fixtures are H=1, where the head-stride error is invisible). Mutating to the logical stride fails it. - Non-vacuity (independently re-run by reviewer): gate-revert, logical-stride, and forced-reused=0 mutations ALL fail. - Honest gating: f16 / non-rank-4 / CPU-in-place-GQA / sink-discontinuous stay `supports_paged_kv=false` → Inc-A non-paged fallback (no silent-wrong paged run). - No regressions: Inc-C #566, Inc-A #565, #541/#543 hybrid, #554 reuse (14/14), native_decode lib (54); scope clean (no standard_attention/GQA kernel, capture core, or `plan_capture_region` edits). ### Remaining (follow-up Inc-D.1) Real 35B-A3B export in **f16** device KV → f16 read-out + lossless paged round-trip; and CPU-in-place-GQA f32 (needs its own H≥2 ORT-oracle fixture — not free). Both correctly gated to non-paged today. ### Reviews Independent opus review, author-lockout enforced: Mary (native-decode specialist) **APPROVE** — 8/8 items verified with reproduced evidence on GPU 0, full mutation battery. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…code (GAP-3 Inc-D.1) (#568) ## GAP-3 Inc-D.1 — f16 device-resident present-KV read-out → paged native CUDA decode Relaxes the Inc-D (`#567`) device paged gate `cuda && f32 && rank==4` → `cuda && (f32||f16) && rank==4`, so **f16 device-resident rank-4 CUDA GQA present-KV** runs **paged** instead of falling to the Inc-A non-paged path. This is the unlock for real fp16 models (e.g. gemma4-e2b, whose decoder KV is confirmed FLOAT16) to decode natively paged. ### How (dtype handling in native read/seed only) - `native_decode/tensor.rs`: `kv_dtype_to_f32` widens f16→f32 via `half` `to_f32_vec` (identical to ORT `to_vec_f32_lossy`); `f32_slice_to_dtype_bytes` narrows f32→f16 via `half::f16::from_f32` (identical to ORT `from_f32_slice_as`) — NOT the logits bit-twiddle. Narrower shared with the embedding-input path (DRY). - `native_decode/cuda.rs`: gate `kv_bindings_paged_rank4`; dtype-branch the read-out + seed. Host paged store stays **f32 for f16 models** (identical to ORT) → the Inc-C/D byte-equality oracle is preserved unchanged. - bf16, CPU-in-place-GQA, non-rank-4, sink-discontinuous stay gated → non-paged fallback (bf16 additionally bails defensively inside the convert). ### Correctness - New `native_paged_prefix_reuse_matches_ort_on_cuda_device_f16`: paged-native-f16 == non-paged-native-cold == ORT-cold tokens; device-mirrored pages **byte-equal** CUDA-vs-ORT (f32 store both sides); `reused=4`. - Convert unit tests: native widen == `half` reference; **f16→f32→f16 bit-exact across all 65536 non-NaN f16 patterns**; f32 identity. - New fixture `tiny-gemma4-vlm-cuda-f16` (Concat-KV so an ORT oracle exists; `value=key*2` bit-exact — the `+0.5` variant was rejected for hitting an f16 round-to-even midpoint). - Non-vacuity (independently re-run by reviewer): gate→f32-only, raw-u16-as-f32 wrong-convert, and mirror-disabled mutations ALL fail. - No regressions: Inc-D #567 f32 path still green, Inc-C #566, Inc-A #565, #541/#543 hybrid, #554 reuse (14/14), 354 lib tests. Scope clean (no attention/GQA kernel, capture core, provider, `CudaGraphLifecycle`, or ORT-bridge edits). ### Remaining (follow-up Inc-D.2) qwen3-30b-a3b is `torch_dtype=bfloat16`; if its ONNX export keeps bf16 KV it stays gated → Inc-D.2 flips the bf16 arm (helpers already have it) after confirming ORT widens bf16→f32 in its paged store. MoE FFN produces no KV (orthogonal); present-KV dtype is the only decode-path gate. ### Reviews Independent opus review with strict author-lockout: Harry **APPROVE** — convert matches ORT exactly, round-trip bit-exact (all 65536 patterns), bf16 excluded, full mutation battery fires, scope clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Standalone follow-up to #533 (Inc3c capture-step-inputs, merged). Validates the captured step-inputs decode path against the real multi-component
inputs_embedsdecoder class, banks measured tok/s, and records the default-on recommendation. Does not touch #533's reviewed code (native_decode/*). Refs #384.Measured (device 4, release, ORT 1.27.0 CUDA, two-length steady-state)
Engagement — counter proof on the real qwen3-0.6b (it DECLINES)
input_ids):from_pipeline_dirrefuses it ("metadata has no pipeline section"), and native-CUDA single-graph decode withONNX_GENAI_NATIVE_DECODER_CAPTURE_STEP_INPUTS=1leavesNATIVE_DECODER_CAPTURED_STEP_INPUT_DECODESat 0 — it captures via the token-idrun_one_tokenlever, never the step-inputs path. So qwen3-0.6b is the wrong class for this flag; the 614/206 table is its single-graph CUDA-graph capture lever (a faithful launch-overhead proxy). Captured still beats ORT 1.42x, eager loses ~2x.inputs_embedsdecoders (qwen3.5-0.8b hybrid, gemma-3n-e2b) are the correct class. gemma-3n's decoder isGroupQueryAttentioncapacity-aware KV → it WOULD engage capture (same class as the synthetic fixture and 35B-A3B). It is gated by two independent, non-Inc3c blockers: (1) the Bool audio-mask clone (canonical fix in Generalize ORT value clone to all POD dtypes (unblocks Bool/gemma-3n audio mask) #540), and (2) required vision inputs for text-only decode (vision poolerOneHotrejects synthetic patches; needs a real image or optional-modality skip).Which engage vs decline
input_idsinputs_embedssmart_resize)inputs_embeds+routedDefault-on recommendation
Safe to default-on for engaging models (byte-identical when it declines, token-parity when it engages, graceful eager fallback everywhere). Blocker to recommending it now: no GREEN real-weights e2e capture number yet (engagement proven only on the synthetic
tiny-gqa-embeds-cudafixture, which faithfully models the GQA-capacityinputs_embedsclass), due to the two unrelated loader/modality gaps — not the optimization. Keep default-off until one realinputs_embedsmodel runs the flag e2e.Artifacts
tests/qwen3_0_6b_capture_step_inputs_decline.rs— concrete counter=0 decline proof.tests/gemma3n_native_cuda_capture_realmodel.rs— forward-looking real-model capture-engagement harness (graceful skip on the vision-input gap; relies on Generalize ORT value clone to all POD dtypes (unblocks Bool/gemma-3n audio mask) #540 for the Bool clone)..squad/decisions/inbox/mary-inc3c-realmodel-capture.md— full finding + measured numbers + recommendation.Verify
cargo fmt --checkclean; clippy x4 (default/native-backend/cuda/cuda,native-backend) clean.cargo test -p onnx-genai-engine --features cuda,native-backend --no-fail-fast: failing set 17, byte-identical to base, 0 regressions.[0,5,6,7], captured 0→3) + qwen3-0.6b decline proof (counter 0) GREEN; gemma3n harness skips gracefully.