Repository navigation
feat(shape-inference): container types inc2 — sequence mutation + tensor⇔sequence conversion (#449) - #486
Conversation
…quence conversion (#449) Increment 2 of the #449 container-type roadmap (foundation PR #477 merged). Adds four opset-11 rules in handlers/container.rs: - SequenceInsert: element type unifies existing element with inserted tensor - SequenceErase: element type preserved - SplitToSequence: tensor→sequence; split-axis symbolic w/ explicit split, else chunk-size-1 with keepdims controlling axis removal - ConcatFromSequence: sequence→tensor (proves the container→tensor seam); concat axis symbolic, new_axis inserts a symbolic stack dim DRY: generalised merge_element→merge_tensor (shared by Construct + Insert), new sequence_element_tensor helper, reused checked_axis. Tensor-only path stays byte-identical (regression test unchanged). Catalog 213→217 ops / 258→262 entries. +21 tests incl. Split→Concat and Construct→Insert→At round-trips. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #486 +/- ##
========================================
Coverage 80.57% 80.58%
========================================
Files 315 315
Lines 122793 122900 +107
Branches 122793 122900 +107
========================================
+ Hits 98944 99037 +93
- Misses 19826 19838 +12
- Partials 4023 4025 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
🔴 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
|
|
VERDICT: APPROVE Independent review by Melina (reviewer). Worktree from origin/squad/449-container-inc2 @ 5d1b256, base main, refs #449. CPU-only, pure shape-inference crate. Author Harry locked out. SummaryClean, spec-faithful increment on the merged #477 container foundation. All four opset-11 ops correct, the byte-identical tensor invariant holds, the DRY generalization is genuinely shared with no divergence, tests are non-tautological, and the catalog pin is exact. Full CPU verification green. 1. Byte-identical tensor path preserved (THE invariant) — CONFIRMED
2. ConcatFromSequence (seq -> tensor) — CORRECT vs ONNXcontainer.rs:238-277.
3. SplitToSequence — CORRECT vs ONNX / runtime referencecontainer.rs:194-234.
4. SequenceInsert / SequenceErase + unify — CONSISTENT with foundation
5. DRY confirmation — CONFIRMED, no divergence
6. Tests non-tautological — CONFIRMED19 new #[test] functions added; real dtype/shape/symbolic assertions, mismatch->error, dtype-only->unresolved, two round-trips (construct+insert+at; split+concat), one parameterized over dtypes. Not mere non-crash checks. 7. Catalog exactness — EXACT
Verification (CPU-only) — ALL GREEN
Minor note (non-blocking)The PR description says op_rules +21 (254->275); the actual delta is 19 new #[test] fns (256->275). A description/count inaccuracy only — the authoritative harness reports 275 passing and the catalog pin is correct. No action required. |
…#527) **Follows #486** (inc2 sequence mutations + conversions, now **merged**). Rebased onto `main` since #486's branch was deleted on merge. refs #449. CPU-only, pure shape-inference crate. ## Summary Increment 3a of the #449 container-type roadmap: `ValueType` (Sequence/Optional/Map) container types now thread through **If** branches and **Loop** carried dependencies — the two headline control-flow cases where sequences are actually used in real models (an If returning a sequence from both branches; a Loop carrying a sequence accumulator). ## What's implemented - **If**: output container type = `unify(then_out, else_out)` via a new recursive `unify_value_type` (Tensor⨝Tensor at the leaves reusing the foundation's `unify_tensor_type`; Sequence/Optional recurse; Map requires an equal key dtype). Element-extent disagreement → fresh symbol; element dtype disagreement → error; one branch container + other tensor → error. - **Loop**: a container carried operand seeds the body formal input (`body_container_seeds`); the body threads/mutates it (`SequenceInsert`/`Identity`/passthrough); the carried body-output container type flows to the Loop output, remapped to the parent symbol space. Loop scan outputs stack tensors → never containers. - `InferenceReport.containers` is the observation channel for tests **and** the body→parent threading channel. `infer_graph_scoped` gains a `seed_containers` param and raises the child interner floor above seeded container symbols. New `CfOutput::Container` variant. ## DRY Promoted the foundation's `merge_tensor`/`merge_shape` from `handlers/container.rs` to `context.rs` as `unify_tensor_type`/`merge_element_shape` (shared by the container handlers and `infer.rs`). Container symbol remap mirrors the exact per-dim rule the tensor CF path uses. ## Byte-identical tensor path All container work is gated on a non-empty container map — empty for pure-tensor graphs, no extra `fresh_dim` minted → symbol numbering unchanged. `tensor_only_path_is_byte_identical_after_container_type_model` stays GREEN, as do all existing If/Loop/Scan tensor tests. ## Honest limitation A *seeded* symbolic element dim crossing a Loop body degrades to a fresh parent symbol (container seeds never touch a body IR `Value`, so it's absent from `child_to_parent`) — SOUND, dtype/structure/concrete extents preserved. Covered by `loop_passthrough_preserves_seeded_sequence_dtype`. ## Scoped out - **inc3b**: Scan container state vars (mechanically identical to Loop seeding, rare, no scan-output payoff). - **inc4**: SequenceMap + cross-subgraph container capture (`remap_node_io` sets `value_type: None` today). ## Verification - `cargo test -p onnx-runtime-shape-inference`: graph_inference **27 → 33** (+6 container CF tests); op_rules 275, container 16, doctest 1 — all GREEN; byte-identical regression GREEN. - `cargo fmt --all --check` clean; `cargo clippy -p onnx-runtime-shape-inference --all-targets -D warnings` clean. - `cargo build -p onnx-runtime-session -p onnx-runtime-eager` GREEN. - Catalog counts **unchanged at 217 ops / 262 entries** (inc3a registers no new operators). Design + status notes: `.squad/decisions/inbox/harry-449-inc3-design.md`, `.squad/decisions/inbox/harry-449-inc3.md`. Do NOT merge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e/hybrid wave logs (#532) Scribe round 5. Records the native multi-component CUDA pipeline decode wave (#484/#485/#486/#487/#525) and distills decisions.md 28520→19858 bytes (under the 20480 gate). State-only; no production code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Increment 2 of the #449 container-type roadmap. Foundation (PR #477, the additive
ValueTypelayer +SequenceEmpty/Construct/Length/At) is merged tomain; this builds on it. refs #449.Ops added (domain
"", opset 11,handlers/container.rs)Sequence mutation:
(seq, tensor, [position]) → seq— element type unifies existing element ⨝ inserted tensor (dtype-mismatch → error, extent disagreement → fresh symbol, dtype-only/rank-mismatch → unknown shape).positionis type-irrelevant.(seq, [position]) → seq— element type preserved.Tensor⇔sequence conversion:
(tensor, [split], axis, keepdims) → seq— element dtype = input dtype; explicitsplit→ split-axis symbolic; no split → chunk size 1 withkeepdimsdeciding keep-at-1 vs remove-axis. Matches runtimesequence/split.rs.(seq, axis, new_axis) → tensor— the sequence→tensor direction of the seam. Recovers a tensorTypeInfofrom the element type: concat axis symbolic;new_axis=1inserts a symbolic stack dim (rank +1);axismandatory. dtype-only element → unresolved (unknown rank), same asSequenceAt.DRY
merge_element→merge_tensor(ctx, op, acc, other), now shared bySequenceConstruct+SequenceInsert;merge_shapereused unchanged.sequence_element_tensor(ctx, i)reader shared by Insert/Concat.handlers::checked_axisreused;output_rank = rank + new_axismakes one axis check cover concat and stack.Guarantees
tensor_only_path_is_byte_identical_after_container_type_modelunchanged & GREEN. No tensor handler touches the container layer.cargo fmt --check,clippy -D warningsclean;onnx-runtime-session/-eagerbuild.Scoped OUT → inc3+
SequenceMap (needs subgraph/body type-threading = container-aware control flow), plus Optional/Map op rules. See
.squad/decisions/inbox/harry-449-inc2.md.CPU-only, pure shape-inference crate work. Draft — do not merge.