Repository navigation
fix(runtime): infer Compress output shapes for large conditions (replaces #1195) - #1253
Merged
Merged
Conversation
A dynamic Compress output shape is the number of selected entries, so the condition tensor has to be read to compute it. The dynamic shape reader admitted only the small tensors that normally carry shape data, so a vision graph whose condition spans a full image failed shape inference with "no inferred shape ... produced by op Compress" and the request 500'd. Read a Compress condition through its own bounded path instead, so the operand it needs is materialized while an unrelated large tensor still is not. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0190e2eb-abe4-451f-b36d-44a035a99b7e
…ndition
The shipped `compress_condition_allows_image_sized_boolean_vectors` test only
exercises the `bounded_compress_condition` predicate; reverting the dispatch
routing that actually wires a `Compress` condition to it leaves that test green,
so it cannot catch a regression of the fix.
Add a behavioural test that builds a `Compress` graph whose boolean condition
initializer is longer than `MAX_SHAPE_DATA_ELEMS` and runs it end to end on the
CPU EP, asserting the data-dependent output shape resolves and the correct rows
are gathered (including the all-false empty and all-true full boundaries).
Reverting the routing makes it fail with `UnresolvedShape { op: "Compress" }`.
Also add explicit boundary coverage for the selected-count sizer: condition
longer than the axis (clamped), shorter than the axis, all-false, all-true.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1253 +/- ##
==========================================
- Coverage 79.95% 79.93% -0.02%
==========================================
Files 360 360
Lines 158518 158790 +272
Branches 158518 158790 +272
==========================================
+ Hits 126741 126936 +195
- Misses 27160 27231 +71
- Partials 4617 4623 +6
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
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
This is #1195 (
fix(runtime): infer Compress output shapes for large conditions) rebased onto currentmain(which now includes #1251, the replacement for its stacked base #1159), plus a follow-up commit that adds the falsifying test the original was missing. Opened as a replacement branch because the originalfix/compress-condition-shapeswas stacked on asplit/*branch, which makesgh pr merge403. Closes #1195.The bug
A dynamic
Compressoutput shape is the count of selected entries, so the executor must read the boolean condition tensor to size the output. The dynamic-shape reader (resolve_node_outputs) admitted a shape input only if it passedbounded_shape_input, whose cap isMAX_SHAPE_DATA_ELEMS = 1024— sized for scalar/vector shape metadata. A vision graph whoseCompresscondition spans a full image is longer than 1024, so it was rejected,dynamic_output_shapesreceived aNonecondition, and the run failed withUnresolvedShape { op: "Compress" }(a 500).The fix
geometry.rs: addbounded_compress_condition— a 1-DBooltensor up toMAX_COMPRESS_CONDITION_ELEMS = 1 << 20.dispatch.rs: route theCompresscondition input (default domain,op_type == "Compress", input index 1) through a newcompress_condition_i64that uses the larger bound, while every other shape input still goes through the ordinaryshape_input_i64. So the one operandCompressgenuinely needs is materialized, and an unrelated large tensor still is not.Test-discipline gap I closed
The PR shipped
compress_condition_allows_image_sized_boolean_vectors, which only asserts thebounded_compress_conditionpredicate. Reverting thedispatch.rsrouting (the behavioural half of the fix) leaves that test green — it cannot catch a regression of the fix. I added:compress_runs_with_image_sized_condition_including_empty_and_full— builds aCompressgraph whoseBoolcondition initializer isMAX_SHAPE_DATA_ELEMS + 500(1524) elements and runs it end to end on the CPU EP, asserting the output shape resolves and exactly the selected rows are gathered. Covers the all-false (empty output) and all-true (full output) boundaries through the real resolve + kernel path.dynamic_output_shapes_compress_boundary_counts— the selected-count sizer at its boundaries: condition longer than the axis (clamped), shorter than the axis, all-false, all-true.Falsification (revert -> RED -> restore -> GREEN)
Reverting only the
dispatch.rsrouting back toshape_input_i64(keeping the helper + method so it stays a clean behavioural RED, not a compile error) makescompress_runs_with_image_sized_condition_including_empty_and_fullfail with exactly:while the shipped predicate-only test and the counting test stay green (confirming they do not cover the routing). Restoring the routing returns all 4 Compress tests to green.
Validation
cargo test -p onnx-runtime-session --lib: 185 passed, 0 failed on currentmain.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com