Honor interleave in concat's value_inference - #2788
Open
LeSingh1 wants to merge 1 commit into
Open
Conversation
concat.value_inference always calls np.concatenate and never reads
self.interleave, so a concat of const inputs with interleave=True folds to the
plain (non-interleaved) concatenation. type_inference gets the shape right, so
nothing catches the mismatch, and common::const_elimination replaces the op
with a const holding the wrong values.
Using the example from the op's own docstring:
in1 = [[1, 2], [3, 4], [5, 6]]
in2 = [[7, 8], [9, 10], [11, 12]]
mb.concat(values=[in1, in2], axis=0, interleave=True)
docstring / runtime: [[1,2],[7,8],[3,4],[9,10],[5,6],[11,12]]
value_inference: [[1,2],[3,4],[5,6],[7,8],[9,10],[11,12]]
All inputs of an interleaved concat share the same shape (type_inference
enforces this), so the interleave is a stack right after the concat axis
followed by folding that new dimension back into the axis. The all-scalar
branch above is left alone: interleaving scalars is the same as stacking them.
The existing interleave coverage
(TestConcat.test_builder_to_backend_stress_interleave) feeds placeholders, so
value_inference never runs there.
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.
Problem
concat.value_inferencealways callsnp.concatenate(values, axis=self.axis.val)and never readsself.interleave. So aconcatof const inputs withinterleave=Trueconst-folds to the plain, non-interleaved concatenation.Using the example spelled out in the op's own docstring:
[[1,2],[7,8],[3,4],[9,10],[5,6],[11,12]]value_inference[[1,2],[3,4],[5,6],[7,8],[9,10],[11,12]]type_inferencecomputes the correct shape(6, 2), so nothing flags the mismatch, andcommon::const_eliminationthen replaces theconcatwith aconstholding the wrong values — the compiled model computes the wrong answer at runtime:Fix
All inputs of an interleaved
concatshare the same shape (type_inferenceenforces exactly this), so interleaving alongaxisis astackimmediately afteraxisfollowed by folding that new dimension back intoaxis.The all-scalar branch above is deliberately left alone: interleaving N scalars and stacking them produce the same result.
I checked the folded value against a slice-assignment reference (
out[k::N] = values[k]along the axis) for ranks 1–3, every positive and negative axis, and 2 or 3 inputs — all match.Tests
In
TestConcat(ops/tests/iOS14/test_tensor_operation.py):test_builder_eval_interleave[axis=0,1,-1,-2]— checks the folded value against the slice-assignment reference, including negative axes.test_builder_eval_interleave_docstring_example— the exact example from the op docstring.All five fail on
mainand pass with the fix.The existing interleave coverage,
TestConcat.test_builder_to_backend_stress_interleaveinops/tests/iOS14/test_tensor_transformation.py, feedsmb.placeholderinputs, sovalue_inferencenever runs there — which is why this went unnoticed.I ran all of
ops/tests/iOS14/test_tensor_operation.pyand the concat-related pass tests before and after; the failure sets are identical (this environment cannot load CoreML.framework, so therun_compare_builder/assert_model_is_validtests fail there either way).