Repository navigation
[Fix][Relax][Frontend][ONNX] Squeeze: raise an error when axes select a non-unit dimension - #20211
Closed
siyiweigeHEW wants to merge 2 commits into
Closed
siyiweigeHEW wants to merge 2 commits into
siyiweigeHEW wants to merge 2 commits into
Conversation
…-unit dimension ONNX Squeeze spec requires an error to be raised when an axis selects a shape entry not equal to one. The relax ONNX frontend delegated to relax.op.squeeze, whose shape inference silently skips non-unit dims (PyTorch semantics) and returned the input unchanged, diverging from onnxruntime and onnx.reference. Validate statically-known axes against the input shape before delegating and raise a ValueError (same message as np.squeeze / onnx.reference) for non-unit dims. Dimensions only known at runtime keep the previous behavior.
Contributor
|
Fixed in #20188, sorry I didn't know you were working on it |
Member
|
Closed as already fixed. Thanks @siyiweigeHEW @OmarAzizi |
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: #20185
Summary
The Relax ONNX frontend silently accepted a
Squeezewhoseaxesselect adimension with size not equal to 1, returning the input unchanged with no
error. The ONNX Squeeze spec
(opset 11+ and earlier) requires the opposite: "If an axis is selected with shape
entry not equal to one, an error is raised." Both onnxruntime and
onnx.referencereject such models; TVM diverged. The converter now validatesstatically-known axes against the input shape and raises a
ValueErrorbeforedelegating to
relax.op.squeeze.Root cause
Squeeze._squeeze(python/tvm/relax/frontend/onnx/onnx_frontend.py) delegatesthe non-constant path directly to
relax.op.squeeze(data, list(axis)). Theunderlying op's shape inference
(
src/relax/op/tensor/manipulate.cc:1269-1284) deliberately implements PyTorchsemantics — "If a dimension is not 1, silently skip it (no-op), matching PyTorch
behavior" (and carries a
Todo(relax-team): revisit here for better check on if the axis being squeezed has length 1). So a non-unit selected axis was silentlyignored and the input returned unchanged. The ONNX frontend performed no
dim == 1check — unlike its constant path, wherenp.squeezealready raisesthe same
ValueErrorasonnx.reference.Fix
Validate before delegating: when
axisis statically known (a tuple) and theinput shape is statically known, every selected (normalized, deduplicated,
bounds-checked) axis must have size 1:
_validate_axes_unit_dimsreuses_normalize_constant_axes(negative-axisnormalization, bounds and uniqueness checks) and raises the same
ValueErrormessage as
np.squeeze/onnx.referencewhen a known dim is!= 1. Dimensionsthat are only known at runtime cannot be validated at import time and keep the
previous behavior — the shared
relax.op.squeezeis intentionally left unchangedbecause other frontends (e.g. PyTorch) rely on its no-op semantics.
Validation
Differential test: Relax (build +
VirtualMachine) vs onnxruntime on the samemodel, cross-checked with
onnx.reference(which is the reference for therejection semantics and for fp64), across valid, invalid-axis, constant, and
out-of-bounds cases.
(2,3)axes=[0](non-unit)ValueError(1,2,3)axes=[1](non-unit)ValueError(1,2,1,3)axes=[1](only non-unit)ValueError(2,3,1)axes=[0,2](mixed)ValueError(2,3)axes=[-1](neg, non-unit)ValueError(1,2,1,3)axes=[0,2](valid, regression)(2,3)(2,3)(2,3)max|diff|=0(1,2,1)axes=[-1](valid neg, regression)(1,2)(1,2)(1,2)max|diff|=0(1,)axes=[0](scalar output, regression)()()()max|diff|=0Totals: 25/25 OK (pre-fix: 6/6 legal models diverged — onnxruntime/ref
rejected, TVM silently returned the input unchanged). Valid axes (incl. negative
axes, scalar output, opset 13/21), the constant-data path, and out-of-bounds
rejection are unchanged. Run:
New in-tree tests cover the raise behavior for both opset 11 (axes attribute) and
opset 13 (axes input), the constant path, and numeric correctness of valid axes
vs onnxruntime.
Files changed
python/tvm/relax/frontend/onnx/onnx_frontend.py—Squeeze._squeeze: validatethat every selected axis has a size-1 dimension (ONNX spec) before delegating to
relax.op.squeeze.tests/python/relax/test_frontend_onnx.py— addtest_squeeze_non_unit_axis_raises,test_squeeze_constant_non_unit_axis_raises,test_squeeze_valid_axes_correctness.