[Fix][Relax][Frontend][Torch] Validate dim in the squeeze converter - #20322
Open
siyiweigeHEW wants to merge 1 commit into
Open
[Fix][Relax][Frontend][Torch] Validate dim in the squeeze converter#20322siyiweigeHEW wants to merge 1 commit into
dim in the squeeze converter#20322siyiweigeHEW wants to merge 1 commit into
Conversation
`_squeeze` dropped out-of-range axes from a list/tuple `dim` and fell back to
`dim=None` when the filtered list turned out empty. For an all-out-of-range
tuple such as `squeeze((5,))` this silently reinterpreted the call as
`squeeze(None)`, removing every size-1 dimension, where native PyTorch raises
`IndexError`:
shape=(2, 1, 3) squeeze((5,)) torch=IndexError tvm=OK (2, 3)
The other argument forms were rejected, but only by the C++ side of
`relax.op.squeeze`, with an opaque op-level error:
shape=(2, 3) squeeze(5) torch=IndexError tvm=InternalError
shape=(2, 3) squeeze(-5) torch=IndexError tvm=InternalError
shape=(2, 3) squeeze((-5,)) torch=IndexError tvm=InternalError
The filter's only observable effect was to drop axes with `d >= rank`, so
replace it with an explicit out-of-range check on all argument forms. The
converter is shared by `from_fx` and `from_exported_program` and is dispatched
from `squeeze`, `squeeze.dim` and `squeeze.dims`, so one check covers them all
and reports the offending axis instead of leaving it to the op.
Add a regression test.
Reported in apache#20321.
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.
Fixes: #20321
Summary
The Relax PyTorch frontend's
_squeezeconverter(
python/tvm/relax/frontend/torch/base_fx_graph_translator.py) reads thedim/dimsargument of a
squeezecall, drops out-of-range axes from a list/tuple, and fallsback to
dim=Nonewhen the filtered list turns out empty:For an all-out-of-range tuple such as
squeeze((5,)),valid_dimsis empty,dimbecomes
None, and the call is silently reinterpreted assqueeze(None)— removeevery size-1 dimension. Native PyTorch raises
IndexErrorfor the same model, so thefrontend turns an invalid model into a differently-shaped one instead of reporting the
bad axis:
The other argument forms already fail, but only on the C++ side of
relax.op.squeeze,with an opaque op-level error rather than a frontend one:
This PR replaces the filter with an explicit range check that rejects the out-of-range
axis with a clear
ValueError, on every argument form.Root cause
The filter's only observable effect was to drop axes with
d >= rank(ford < 0,len(shape) + d < len(shape)is always true, so negative axes were never dropped). So:d >= rankwere silently dropped, and dropping all of them flipped themeaning of the call to "squeeze everything";
d < -rankwere kept and handed torelax.op.squeeze, which rejected themwith an
InternalError;dimnever went through the filter at all, so it always took theInternalErrorpath.Both outcomes are wrong for the frontend: torch rejects the model outright, and the
converter is the last place that can report which axis is bad. The comment above the
filter ("filter out axes where dimension is not 1") also does not describe what the code
does — it never inspects the dimension size, only the bounds.
Fix
python/tvm/relax/frontend/torch/base_fx_graph_translator.py—_squeeze:Notes:
isinstance(d, int)guard means the check only applies to literal integer axes,so a non-literal axis is forwarded unchanged — the fix cannot reject anything the old
code accepted other than genuinely out-of-range integer axes;
dimis no longer rewritten, so a tuple is passed through as-is (relax.op.squeezeaccepts both tuples and lists) and an empty tuple keeps its existing no-op meaning;
_squeezeis shared byfrom_fxandfrom_exported_program, and is dispatched fromsqueeze,squeeze.dimandsqueeze.dims, so a single change covers all of them andmakes the message consistent with the range check
relax.op.squeezealready performs.Validation
In-tree regression test (added)
test_squeeze_out_of_range_dimintests/python/relax/test_frontend_from_fx.py:dim ∈ {(5,), 3, -4, (-4,), (0, 5)}on a(1, 2, 1)input withValueError: squeeze dim <d> is out of range ...— this covers a fullyout-of-range tuple, a mixed tuple, and the scalar forms;
squeeze((0, 2))on(1, 2, 1)stilllowers to
R.squeeze(inp_0, axis=[0, 2])(asserted structurally viaverify_model).The test fails without the fix with
Failed: DID NOT RAISE <class 'ValueError'>on theheadline
(5,)case, and passes with it.No test is added for
from_exported_program:torch.exportrejects an out-of-rangedimat trace time withIndexError, so that path never reaches_squeezewith a badaxis.
Behaviour after the fix
In-range behaviour is unchanged; the control cases from the existing
test_squeezeandfrom a direct differential against native PyTorch all still match:
Full-suite run
tests/python/relax/test_frontend_from_fx.pyandtests/python/relax/test_frontend_from_exported_program.pyrun in full (373 passed,16 failed, 2 skipped). The 16 failures are the same pre-existing, unrelated ones seen on
the unmodified tree (
test_extended_unary_ops,test_interpolate,test_select_slice,test_masked_select,test_to_copy,test_index_put,test_eye,test_cross_entropy,the
test_dynamic_shape*family,test_sym_size_int,test_stochastic_depth); thefailure set is unchanged, so there are no regressions.
Files changed
python/tvm/relax/frontend/torch/base_fx_graph_translator.py— replace theaxis-dropping filter in
_squeezewith an out-of-range check.tests/python/relax/test_frontend_from_fx.py— addtest_squeeze_out_of_range_dim.