[Relax][Frontend][ONNX] Add support for Pad mode="wrap" for opset 19#19827
[Relax][Frontend][ONNX] Add support for Pad mode="wrap" for opset 19#19827napronald wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for ONNX Pad operator version 19 (_impl_v19) in the Relax frontend, enabling the "wrap" padding mode (circular padding). Unit tests are updated to include a test case for this new mode using opset=19. The feedback suggests refactoring _impl_v19 to delegate non-wrap modes to _impl_v11 to avoid substantial code duplication and improve maintainability.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| @classmethod | ||
| def _impl_v19(cls, bb, inputs, attr, params): | ||
| pads = get_constant(inputs[1], params) | ||
| constant_value = get_constant(inputs[2], params) | ||
| if constant_value is not None: | ||
| constant_value = constant_value.data.numpy().item() | ||
| else: | ||
| constant_value = 0.0 | ||
|
|
||
| if isinstance(pads, relax.Constant): | ||
| pad_before, pad_after = _np.split(pads.data.numpy(), 2) | ||
| pad_before = _np.ndarray.tolist(pad_before) | ||
| pad_after = _np.ndarray.tolist(pad_after) | ||
| else: | ||
| raise ValueError("Dynamic pads are not supported yet.") | ||
|
|
||
| pad_mode = attr.get("mode", b"constant").decode("utf-8") | ||
| if pad_mode not in ["constant", "edge", "reflect", "wrap"]: | ||
| raise tvm.error.OpAttributeInvalid( | ||
| "Value " + pad_mode + ' in attribute "mode" is invalid for operator Pad.' | ||
| ) | ||
|
|
||
| if pad_mode == "constant": | ||
| return bb.emit_te(topi.nn.pad, inputs[0], pad_before, pad_after, constant_value) | ||
| elif pad_mode == "reflect": | ||
| return bb.emit_te(topi.nn.mirror_pad, inputs[0], pad_before, pad_after, "REFLECT") | ||
| elif pad_mode == "wrap": | ||
| return bb.emit_te(topi.nn.circular_pad, inputs[0], pad_before, pad_after) | ||
| else: | ||
| # edge mode - replicate border values | ||
| return bb.emit_te(topi.nn.replicate_pad, inputs[0], pad_before, pad_after) | ||
|
|
There was a problem hiding this comment.
The implementation of _impl_v19 is almost identical to _impl_v11, introducing significant code duplication. Since the only difference is the support for mode="wrap", we can simplify _impl_v19 by handling the "wrap" mode directly and delegating all other modes to _impl_v11. This reduces duplication and improves maintainability.
@classmethod
def _impl_v19(cls, bb, inputs, attr, params):
pad_mode = attr.get("mode", b"constant").decode("utf-8")
if pad_mode == "wrap":
pads = get_constant(inputs[1], params)
if isinstance(pads, relax.Constant):
pad_before, pad_after = _np.split(pads.data.numpy(), 2)
pad_before = _np.ndarray.tolist(pad_before)
pad_after = _np.ndarray.tolist(pad_after)
else:
raise ValueError("Dynamic pads are not supported yet.")
return bb.emit_te(topi.nn.circular_pad, inputs[0], pad_before, pad_after)
return cls._impl_v11(bb, inputs, attr, params)There was a problem hiding this comment.
Sorry for the late reply
I did a search and found that Pad opset 19 supports the optional 4th input axes. When axes is provided, pads has shape [2 * len(axes)], not [2 * rank(data)].
For example, this is valid ONNX:
input rank = 3
axes = [2]
pads = [2, 2]
mode = "wrap"
It means “pad only axis 2 by 2 before and 2 after”. But _impl_v19 currently ignores inputs[3], splits pads directly, and passes:
pad_before = [2]
pad_after = [2]
to topi.nn.circular_pad, which expects pad lists matching the full input rank. So this valid Pad-19 model will either fail with a pad length mismatch or be handled incorrectly.
Could we either support axes by expanding the pads to full-rank form, e.g.
pad_before = [0, 0, 2]
pad_after = [0, 0, 2]
or explicitly reject axes with a clear error? Please also add a test for mode="wrap" with axes so this path is covered.
62f4bdf to
f166719
Compare
|
Updated.
I added a |
2606954 to
d08e63b
Compare
tlopex
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the contribution!
|
Could you please resolve the conflict and change the test to structural check? Thanks! |
d08e63b to
7f507ed
Compare
|
Resolved and updated the test to use structural checks. |
Summary
The ONNX Pad operator introduced
mode="wrap"(circular padding) in opset 19. Currently, the Relax ONNX frontend has no support for opset 19, which raisesChanges
Add opset 19 handling to the Pad converter that dispatches
mode="wrap"to topi.nn.circular_pad, which already implements circular padding but was never wired up to the ONNX frontend. Existing behavior for earlier Pad opsets is unchanged.Reproduce