[Relax][ONNX] Support Resize dynamic ROI via TOPI#18963
Merged
tlopex merged 2 commits intoapache:mainfrom Apr 2, 2026
Merged
Conversation
The ONNX Resize converter previously rejected non-constant ROI inputs, which blocked models where ROI is provided at runtime. This change adds a dynamic-ROI path lowered through TOPI resize kernels while preserving the existing relax.image.resize* path for static ROI. Specifically: - add reusable helper to convert ONNX full ROI ([starts..., ends...]) into spatial ROI vector - add reusable helper to emit topi.image.resize1d/2d/3d for dynamic ROI - keep static ROI fast path for relax.image.resize2d/resize3d - normalize dynamic ROI expr before emit_te to ensure struct_info is populated - handle optional Resize inputs (roi/scales/sizes) more defensively - add frontend test coverage with graph-input ROI: test_resize_dynamic_roi_tf_crop_and_resize Ref: apache#18945
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces support for dynamic Region of Interest (ROI) in the ONNX Resize operator by leveraging TOPI's dynamic-ROI path. It adds helper functions to map ONNX ROI to spatial ROI and emit TOPI resize operations for 1D, 2D, and 3D data. The review feedback identifies a bug in the ROI index mapping for 3D resizing, where the order must be adjusted to match TOPI's expectations, and points out an invalid assertion that always evaluates to true and should be replaced with a proper exception.
tlopex
requested changes
Apr 1, 2026
Member
tlopex
left a comment
There was a problem hiding this comment.
The current test only covers the 2D case. Please add at least a 3D dynamic ROI test as well
Contributor
Author
|
cc @tlopex |
- Reorder ONNX NCDHW spatial ROI (D,H,W) to TOPI resize3d (W,H,D) for emit_te dynamic path; apply the same mapping for static 5D ROI to relax.image.resize3d. - Replace invalid assert f-string branches with ValueError for unsupported scales/sizes types. - Add test_resize_dynamic_roi_3d_tf_crop_and_resize with explicit full-tensor ROI inputs for stable ORT vs TVM comparison. Ref: apache#18945
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.
The ONNX Resize converter previously rejected non-constant ROI inputs, which blocked models where ROI is provided at runtime. This change adds a dynamic-ROI path lowered through TOPI resize kernels while preserving the existing relax.image.resize* path for static ROI.
Specifically:
Ref: #18945