-
Notifications
You must be signed in to change notification settings - Fork 84
Do not try to fold op.SplitToSequence when split is None
#2550
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
Do not try to fold op.SplitToSequence when split is None
#2550
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2550 +/- ##
==========================================
+ Coverage 70.37% 70.49% +0.11%
==========================================
Files 218 218
Lines 26428 26430 +2
Branches 2646 2647 +1
==========================================
+ Hits 18600 18633 +33
+ Misses 6922 6889 -33
- Partials 906 908 +2 ☔ View full report in Codecov by Sentry. |
| """ | ||
| input = node.inputs[0] | ||
| if len(node.inputs) == 1: | ||
| # split is not provided |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When split is not provided it is a scalar 1. Can we use this info?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it? The issue @xadupre brought up was only one input. model id: facebook/sam-vit-base, so it hit indexerror.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think at least according to the spec: https://onnx.ai/onnx/operators/onnx__SplitToSequence.html#summary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| return op.SplitToSequence(self, axis=dim, keepdims=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably came from here
split is an optional input to op.SplitToSequence.