Make squeeze and slice_by_size const folding agree with their type inference - #2803
Open
LeSingh1 wants to merge 1 commit into
Open
Make squeeze and slice_by_size const folding agree with their type inference#2803LeSingh1 wants to merge 1 commit into
LeSingh1 wants to merge 1 commit into
Conversation
…ference squeeze documents, and type_inference implements, PyTorch's rule that an axis whose size is not 1 is ignored instead of raising. value_inference handed the axes straight to np.squeeze, which raises. So squeezing a const along a non-single dimension aborted the conversion, even though the same program converts and runs when the input is not const. squeeze's zero rank result was returned as self.x.val[0], which is only a scalar when x is rank 1. For an all-ones shape of rank 2 or more this returned an array, and building the op failed with "Types should have zero rank ndarray input". Index the squeezed value instead. slice_by_size's value_inference treated any size <= 0 as "the rest of the dimension", while its type_inference (and the docstring) give that meaning to -1 only. A size of 0 therefore produced a const whose value disagreed with its own declared shape.
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.
Three places in
iOS15/tensor_transformation.pywherevalue_inferencedisagrees with thetype_inference(and the docstring) of the same op, so a program behaves differently depending only on whether its input happens to be const.squeezeraises on a non-single dimensionThe docstring says, verbatim:
type_inferenceimplements exactly that (if squeezed_shape[i] == 1:), andTestSqueeze::test_non_single_element_dimpins the runtime behavior.value_inferencepassed the axes straight tonp.squeeze, which is the NumPy rule:That is the docstring's own example, and it aborts the conversion. The same program converts and runs when
xis a model input rather than a const.squeezereturns a rank 1 array where the type says scalarself.x.val[0]is a scalar only whenxis rank 1. For an all-ones shape of rank 2 or more it is still an array, so building the op fails:Fixed by indexing the squeezed value with
(), which gives the same numpy scalar the rank 1 case already produced.slice_by_sizetreats a size of 0 as "the rest of the dimension"The docstring gives that meaning to
-1only, andtype_inferenceagrees (elif s != -1: ret_shape.append(s)). Withsize = [0, 2]on a(3, 4)input andbegin = [1, 1],type_inferencereports shape(0, 2)whilevalue_inferencereturns a(2, 2)array — a const whose value contradicts its own declared type. Changed the guard to!= -1so the two agree.Testing
Three new builder-eval tests, all of which fail on
main:TestSqueeze::test_builder_eval_non_single_element_dimuses the same axes as the existingtest_non_single_element_dimbackend test, so the folded value is pinned to the runtime value.TestSqueeze::test_builder_eval_rank_0_from_higher_rankextends the existingtest_builder_eval_rank_0to ranks 2 and 3.TestSliceBySize::test_builder_eval_zero_sizechecks the folded shape against the declared shape.The existing
TestSqueeze/TestSliceBySizetests are unchanged and still pass, includingtest_squeeze_value_inference_is_inplace(the folded value is still a view of the const) andtest_builder_eval_rank_0. The full iOS14/16/17/18test_tensor_transformation.pysuites pass.