Skip to content

[Relax][ONNX] Update ReduceL1 to opset 18 #18072

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 23, 2025
Merged

Conversation

vacu9708
Copy link
Contributor

@vacu9708 vacu9708 commented Jun 17, 2025

Summary

This PR resolves #18032 where a valid onnx model causes an error in TVM but not in onnxruntime.
Why the error occurs

  1. Starting in ReduceL1 - 18, axes is passed as an input rather than an attribute
  2. ReduceL1-13 receives a None axes and reduces all dimensions to size 1.
  3. In DepthToSpace, dimension size(1) // block size(2) yields zero. As a result manipulate.cc:860 becomes false and an error is triggered at manipulate.cc:865.

Anyway in summary, the error occurs because of the updated ReduceL1.

Changes

  • Update ReduceL1 from opset 13 to opset 18.
  • Add corresponding test cases to cover the new behavior.

Differences vs opset<18:

  • axes is now an input instead of an attribute.
  • Introduces attribute noop_with_empty_axes
Axes provided? noop_with_empty_axes Behavior
No 0 Reduce over all dimensions
No 1 Return the input unchanged
Yes any Reduce over the specified axes

To do

  • All other reduce ops in the ONNX spec have also been updated in the same way
    • I’ve already updated those reduce ops in TVM and will open a pull request for them once this one is merged
  • The reduce ops(relax.op.max/min/sum, etc) in TVM currently support only constant axes
    • They need to support dynamic axes for full ONNX spec compliance

Implementation in TVM

The official ONNX spec ambiguously describes "empty axes" — it’s unclear whether it refers to None or to an empty list [ ]. However, the "Inputs" section states there can be one or two inputs and that axes is optional, thus I think interpreting "empty axes" as either None or [ ] aligns with the specification.

Notes on a bug in ONNX Runtime

Opened issue microsoft/onnxruntime#25095

Axes list noop_with_empty_axes Behavior Correct behavior
[ ] 1 Returns the input unchanged
[ ] 0 Reduces all dimensions
None 1 Raises a runtime error Returns the input unchanged
None 0 Reduces all dimensions

@vacu9708 vacu9708 force-pushed the reduceL1 branch 2 times, most recently from 59ffa14 to 913b970 Compare June 18, 2025 00:31
- Update ReduceL1-13 to ReduceL1-18
- Add the corresponding test cases
@Hzfengsy Hzfengsy merged commit 8327a8c into apache:main Jun 23, 2025
20 checks passed
@vacu9708 vacu9708 deleted the reduceL1 branch July 9, 2025 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants