[Fix][Relax][Frontend][Torch] Compare shapes without == on symbolic dims in the reshape shortcut - #20377
Open
hiyufan wants to merge 1 commit into
Open
[Fix][Relax][Frontend][Torch] Compare shapes without == on symbolic dims in the reshape shortcut#20377hiyufan wants to merge 1 commit into
== on symbolic dims in the reshape shortcut#20377hiyufan wants to merge 1 commit into
Conversation
… dims in the reshape shortcut `_reshape` skips an identity reshape by comparing the input shape with the target as `list(current_shape) == list(dims)`. On a symbolic dimension `==` builds a PrimExpr instead of answering, and Python then asks it for a truth value: ValueError: Cannot use and / or / not operator to Expr It only surfaces when the ranks match, because list equality compares lengths first. So `x.reshape(x.shape[0], -1)` on a rank-3 input imported fine while `x.reshape(x.shape[0], 0, x.shape[0])` raised; in the reshape sweep from apache#20255 that was 40 of the 938 cases, all with a symbol at a position of the target that lined up with the same symbol in the input. `_same_dims` compares dimension by dimension: static dims as integers, symbolic dims with tvm_ffi.structural_equal, and a static-vs-symbolic pair as different. A genuine identity with a symbolic batch, `x.reshape(x.shape[0], 2, 4)` on `(batch, 2, 4)`, is still recognised and emits no reshape; an expression written differently on the two sides (`s*2` vs `2*s`) is treated as different, which costs a no-op reshape and never a wrong shape. Reshape sweep (938 targets over inputs that mix symbolic and zero dims, output shape compared with torch.export's, symbols canonicalised): before 892 matched 46 mismatched (40 raising, 6 the sweep's renderer) after 932 matched 6 mismatched ( 0 raising, 6 the sweep's renderer) no case fails after this change that did not fail before it; 40 repaired Test: `x.reshape(x.shape[0], 0, x.shape[0])` on a dynamic `(batch, 0, 4)` input builds and matches torch, and `x.reshape(x.shape[0], 2, 4)` on `(batch, 2, 4)` emits no reshape. Fails against the previous head with the ValueError above.
cchung100m
approved these changes
Sep 20, 2026
cchung100m
left a comment
Contributor
There was a problem hiding this comment.
LGTM, thanks to @hiyufan 😄
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
_reshapeskips an identity reshape withOn a symbolic dimension
==builds aPrimExprinstead of answering, and Python then asks it for a truth value:It only surfaces when the ranks match, because list equality compares lengths first — which is why
x.reshape(x.shape[0], -1)on a rank-3 input imports andx.reshape(x.shape[0], 0, x.shape[0])raises:mainx.reshape(x.shape[0], 0, x.shape[0])(batch, 0, 4)(batch, 0, batch)ValueErrorx.reshape(0, x.shape[1], x.shape[1])(0, batch, 4)(0, batch, batch)ValueErrorx.reshape(x.shape[0], -1)(batch, 0, 4)(batch, 0)This is the "40 pre-existing
ValueErrorcases" I reported as out of scope in #20255 — I had assumed they were below the frontend. They are this line.Fix
_same_dimscompares dimension by dimension: static dims as integers, symbolic dims withtvm_ffi.structural_equal, a static-vs-symbolic pair as different. A genuine identity with a symbolic batch is still recognised (x.reshape(x.shape[0], 2, 4)on(batch, 2, 4)emits no reshape, asserted in the test). An expression written differently on the two sides (s*2vs2*s) is treated as different, which costs a no-op reshape and never a wrong shape.Verification
The reshape sweep from #20255 (938
reshape/view/flatten/unflattentargets over inputs mixing symbolic and zero dims, output shape compared withtorch.export's, symbol names canonicalised):mainPer-case diff: no case fails with this change that did not fail before it; 40 repaired. The 6 left are the sweep's own renderer printing
s*2and2*sdifferently (verified in #20255's thread), not the frontend.test_frontend_from_exported_program.pyandtest_frontend_from_fx.py: failure sets identical before and after apart from the new test.ruff check/ruff format --check(v0.12.3) clean.Test
test_reshape_symbolic_target_same_rank— the first row above builds and matches torch under a dynamic batch, and the identity case emits no reshape. Fails against the previous head with theValueErrorabove.Independent of #20372–#20376 (branched from
main). Touches the same_reshapeas #20255 (merged), one line below it.This change was prepared with AI assistance (Claude). I have reviewed and verified it, and can speak to it in review.