Skip to content

Conversation

@justinchuby
Copy link
Collaborator

@justinchuby justinchuby commented Sep 29, 2025

Fixes #2577

Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Copy link
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

This PR fixes collapse slices rewrite rules to properly handle unknown dimensions by updating the onnx-ir dependency from 0.1.9 to 0.1.10. The fix addresses issue #2577 where the slice optimization logic incorrectly processed tensors with unknown dimension sizes.

  • Updates onnx-ir dependency to version 0.1.10 to access new functionality
  • Adds unknown dimension check in slice shape comparison logic
  • Ensures slice optimization is only applied when tensor dimensions are known

Reviewed Changes

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

File Description
pyproject.toml Updates onnx_ir dependency minimum version to 0.1.10
noxfile.py Updates ONNX_IR constant to match new version requirement
onnxscript/rewriter/rules/common/_collapse_slices.py Adds unknown dimension safety check in _same_shape function

@justinchuby justinchuby added this to the 0.5.3 milestone Sep 29, 2025
@codecov
Copy link

codecov bot commented Sep 29, 2025

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.07%. Comparing base (7227655) to head (6ecebdc).
⚠️ Report is 6 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...nxscript/rewriter/rules/common/_collapse_slices.py 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2583   +/-   ##
=======================================
  Coverage   70.07%   70.07%           
=======================================
  Files         223      223           
  Lines       26266    26268    +2     
  Branches     2602     2603    +1     
=======================================
+ Hits        18405    18407    +2     
  Misses       6950     6950           
  Partials      911      911           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@justinchuby justinchuby force-pushed the justinchu/fix-slice-rewrite branch from c5f6df6 to 62de1f0 Compare September 30, 2025 19:28
@github-project-automation github-project-automation bot moved this from Todo to Done in ONNX Script Review Board Oct 1, 2025
@justinchuby justinchuby merged commit 149d567 into main Oct 1, 2025
31 of 32 checks passed
@justinchuby justinchuby deleted the justinchu/fix-slice-rewrite branch October 1, 2025 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

Wrong graph optimization with dynamic shape

2 participants