Skip to content

Improve checks in concat_slice_elimination against optional attrs and unsqueeze_elimination against invalid model#27638

Merged
yuslepukhin merged 7 commits intomainfrom
yuslepukhin/linerregressor_oob
Mar 19, 2026
Merged

Improve checks in concat_slice_elimination against optional attrs and unsqueeze_elimination against invalid model#27638
yuslepukhin merged 7 commits intomainfrom
yuslepukhin/linerregressor_oob

Conversation

@yuslepukhin
Copy link
Copy Markdown
Member

This pull request introduces improvements to the concat_slice_elimination and unsqueeze_elimination optimizer passes, focusing on correctness, robustness, and code clarity. The changes include enhanced handling of optional Slice operator attributes, stricter validation of axes and steps, and improved error handling for invalid model inputs.

Improvements to concat_slice_elimination:

  • Materialized default values for optional axes and steps in the Slice operator, ensuring safe indexing and alignment with ONNX defaults. (onnxruntime/core/optimizer/concat_slice_elimination.cc)
  • Refined the fusion pattern to only allow starts.size() == 1, clarifying the scope of the optimization and preventing incorrect fusions. (onnxruntime/core/optimizer/concat_slice_elimination.cc)
  • Added <numeric> include to support new code using std::iota. (onnxruntime/core/optimizer/concat_slice_elimination.cc)

Improvements to unsqueeze_elimination:

  • Added validation for axes values, including range checks and detection of duplicate axes, returning errors for invalid models instead of silently proceeding. (onnxruntime/core/optimizer/unsqueeze_elimination.cc)
  • Added core/providers/common.h include for utility functions used in validation. (onnxruntime/core/optimizer/unsqueeze_elimination.cc)

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Improves robustness and correctness of two optimizer passes (ConcatSliceElimination and UnsqueezeElimination) by adding safer handling of optional Slice inputs and stricter validation of Unsqueeze axes, and extends the optimizer unit test suite to cover these scenarios.

Changes:

  • Materialize default axes/steps for Slice in ConcatSliceElimination and tighten the supported fusion pattern to starts.size() == 1.
  • Add out-of-range and duplicate-axis validation to UnsqueezeElimination, returning explicit errors for invalid models.
  • Add targeted optimizer tests for Unsqueeze elimination and Slice optional-input defaulting behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
onnxruntime/test/optimizer/graph_transform_test.cc Adds unit tests for Unsqueeze elimination behavior and for Concat+Slice fusion when Slice omits optional axes/steps.
onnxruntime/core/optimizer/unsqueeze_elimination.cc Adds axis range/dup validation for Unsqueeze elimination and improves robustness against invalid models.
onnxruntime/core/optimizer/concat_slice_elimination.cc Materializes Slice default axes/steps, adds <numeric>, and narrows fusion eligibility to single-dimension slicing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread onnxruntime/core/optimizer/concat_slice_elimination.cc Outdated
Comment thread onnxruntime/test/optimizer/graph_transform_test.cc Outdated
Comment thread onnxruntime/test/optimizer/graph_transform_test.cc Outdated
@yuslepukhin yuslepukhin marked this pull request as ready for review March 12, 2026 23:53
@yuslepukhin yuslepukhin requested a review from edgchen1 March 12, 2026 23:55
Comment thread onnxruntime/core/optimizer/concat_slice_elimination.cc Outdated
Comment thread onnxruntime/test/optimizer/graph_transform_test.cc Outdated
Comment thread onnxruntime/test/optimizer/graph_transform_test.cc Outdated
edgchen1
edgchen1 previously approved these changes Mar 16, 2026
@yuslepukhin
Copy link
Copy Markdown
Member Author

/azp run Linux TensorRT CI

@azure-pipelines
Copy link
Copy Markdown

No pipelines are associated with this pull request.

@yuslepukhin yuslepukhin merged commit d0b7cf2 into main Mar 19, 2026
100 of 103 checks passed
@yuslepukhin yuslepukhin deleted the yuslepukhin/linerregressor_oob branch March 19, 2026 14:35
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.

3 participants