Skip to content

[BugFix][Relax][ONNX] Resolve param Vars in Concat to handle mixed Shape/Tensor inputs#19498

Merged
tlopex merged 2 commits intoapache:mainfrom
swjng:fix/onnx-concat-resolve-param-vars
May 4, 2026
Merged

[BugFix][Relax][ONNX] Resolve param Vars in Concat to handle mixed Shape/Tensor inputs#19498
tlopex merged 2 commits intoapache:mainfrom
swjng:fix/onnx-concat-resolve-param-vars

Conversation

@swjng
Copy link
Copy Markdown
Contributor

@swjng swjng commented May 3, 2026

Description

When from_onnx(model, keep_params_in_input=True) is used, every ONNX initializer becomes a relax.Var instead of a relax.Constant. The Concat handler's is_shape_like() check only recognizes relax.ShapeExpr and 1D-int64 relax.Constant, so a 1D-int64 shape value loaded as a Var is no longer recognized.

When such a Var is concatenated with a ShapeExpr — the standard pattern for dynamic-batch Reshape in PyTorch-exported ONNX models — the heterogeneous Tuple(ShapeExpr, Tensor) is rejected by relax.op.concat with:

InternalError: Op(relax.concat) expects the input to be a Tuple of Tensors.
However, the given input is R.Tuple(R.Shape([N]), R.Tensor((1,), dtype="int64"))

This effectively breaks keep_params_in_input=True for any model with dynamic-batch Reshape (extremely common in PyTorch ONNX exports).

Fix

Run each Concat input through the existing get_constant helper before the is_shape_like check. This resolves any Var that maps to a known param back to its baked Constant, restoring the all-shape-like fast path.

Minimal repro

An 8-node ONNX graph (ShapeSliceConcat([dyn_n, [12]])Reshape) fails with keep_params_in_input=True before this PR and passes after. A regression test (test_concat_with_param_shape_value) covers this pattern.

Testing

pytest tests/python/relax/test_frontend_onnx.py -k concat

9 passed (1 new + 8 existing).

…ape/Tensor inputs

When `from_onnx(model, keep_params_in_input=True)` is used, every ONNX
initializer becomes a `relax.Var` instead of a `relax.Constant`. The
Concat handler's `is_shape_like()` check only recognizes `ShapeExpr`
and 1D-int64 `Constant`, so a 1D-int64 shape value loaded as a Var is
no longer recognized. When such a Var is concatenated with a `ShapeExpr`
(the standard pattern for dynamic-batch Reshape in PyTorch-exported
ONNX models), the heterogeneous Tuple(ShapeExpr, Tensor) is rejected by
`relax.op.concat` with InternalError.

Run each Concat input through the existing `get_constant` helper before
the shape-like check; this resolves any Var that maps to a known param
back to its baked Constant, restoring the all-shape-like fast path.

Adds a regression test exercising the dynamic-batch Reshape pattern
with `keep_params_in_input=True`.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the ONNX frontend's Concat operator to resolve input parameters to their constant values, ensuring that 1D-int64 shape values can correctly follow the shape-like fast path when keep_params_in_input is set to true. This change specifically addresses issues with dynamic-batch Reshape patterns in PyTorch-exported models. A new test case has been added to verify this fix. I have no further feedback to provide.

@tlopex
Copy link
Copy Markdown
Member

tlopex commented May 3, 2026

Hi @swjng
Thanks for the fix!

However, Concat._impl_v13 now calls get_constant on every input unconditionally. get_constant has a side effect: when the input is a param Var, it updates graph_nodes[name] to a baked relax.Constant.

This fixes the shape-construction case, but it can also change the semantics of ordinary tensor concat under keep_params_in_input=True. For example, an ONNX graph with Concat(x, weight) should keep weight as a runtime parameter. With this change, weight may be folded into the IR as a constant, so the compiled function no longer depends on the runtime value passed for that parameter.

Could we narrow the fix so param resolution is used only for the all-shape-like fast path, preferably without mutating _nodes? The tensor fallback should continue using the original inputs. It would also be good to add a regression test for normal tensor Concat(input, initializer) with keep_params_in_input=True, verifying that the initializer remains a real parameter input.

…st path

Address review feedback from @tlopex on apache#19498: the previous fix called
get_constant on every input, which mutates graph_nodes and would fold a
runtime weight into a Constant for ordinary Concat(input, weight) under
keep_params_in_input=True.

Replace with a local non-mutating peek that only resolves a param Var
when it is a 1D int64 tensor, and only feed the resolved values into the
shape-like fast path. The tensor fallback keeps the original inputs so
runtime parameters remain runtime parameters.

Add a regression test for ordinary Concat(input, weight) verifying the
weight stays in main's param list and is detached as a real parameter.
@swjng
Copy link
Copy Markdown
Contributor Author

swjng commented May 3, 2026

Thanks. Narrowed it as suggested in the follow-up commit: a local non-mutating resolve returns a Constant only for 1D-int64 param Vars, and only the all-shape-like fast path consumes the resolved list. The tensor fallback uses the original inputs, so Concat(input, weight) still references weight as a runtime Var.

Added test_concat_with_param_tensor_keeps_runtime_param covering exactly that case (asserts w stays in main's params and is recovered by detach_params).

Copy link
Copy Markdown
Member

@tlopex tlopex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you

@tlopex tlopex merged commit 82a37da into apache:main May 4, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants