-
Notifications
You must be signed in to change notification settings - Fork 56
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
[rewriter] Remove redundant op.Slice and op.ScatterND #1925
Merged
titaiwangms
merged 7 commits into
microsoft:main
from
titaiwangms:titaiwang/add_collapsed_slice_rule
Nov 1, 2024
Merged
[rewriter] Remove redundant op.Slice and op.ScatterND #1925
titaiwangms
merged 7 commits into
microsoft:main
from
titaiwangms:titaiwang/add_collapsed_slice_rule
Nov 1, 2024
Conversation
This file contains 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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1925 +/- ##
========================================
Coverage 75.23% 75.23%
========================================
Files 254 257 +3
Lines 27644 27782 +138
Branches 3225 3242 +17
========================================
+ Hits 20797 20901 +104
- Misses 5877 5900 +23
- Partials 970 981 +11 ☔ View full report in Codecov by Sentry. |
justinchuby
reviewed
Oct 30, 2024
justinchuby
reviewed
Oct 30, 2024
justinchuby
reviewed
Oct 30, 2024
justinchuby
reviewed
Oct 30, 2024
justinchuby
reviewed
Oct 30, 2024
justinchuby
reviewed
Oct 30, 2024
justinchuby
reviewed
Oct 30, 2024
justinchuby
reviewed
Oct 30, 2024
justinchuby
reviewed
Oct 30, 2024
justinchuby
approved these changes
Oct 31, 2024
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.
Fixes https://github.com/microsoft/onnx-converters-private/issues/270
It is observed that ExportedProgram could generates aten::slice.Tensor and aten::slice_scatter.default that slice nothing:
The slices would result in redundant op.Slice ops in ONNX graph that does nothing, and op.ScatterND that basically replaces the whole input to updates, which takes a lot of time in inference.
This rule set recognizes the redundant slices by checking if the following requirements are met:
(1) starts = 0
(2) ends >= inputs[dim].shape or ends == _INT64_MAX
(3) steps == 1
This rule set recognizes the redundant scatterND by checking if the following requirements are met:
(1) indices has the same length as the first dim of input
(2) indices is from 0 to input.shape[0]
(3)input has the same shape as updates
Benchmark on ghostnet_100 (the original speed up was 0.0256):