-
Notifications
You must be signed in to change notification settings - Fork 72
Improves aten_chunk conversion #2434
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2434 +/- ##
==========================================
+ Coverage 69.23% 69.25% +0.02%
==========================================
Files 210 210
Lines 25389 25381 -8
Branches 2546 2546
==========================================
+ Hits 17577 17578 +1
+ Misses 6939 6929 -10
- Partials 873 874 +1 ☔ View full report in Codecov by Sentry. |
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.
The original code references on torchscript symbolic function implementation. Could you explain why the op could work on this way? And could you update the code, because g does not make any sense.
return op.SplitToSequence(self, list_split, axis=dim) | ||
if chunks == 1: | ||
return g.op.Identity(self) | ||
return g.op.Split(self, axis=dim, num_outputs=chunks) |
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.
Thanks - the change looks good except for the use of g. Could you update usage? We can then merge
return op.SplitToSequence(self, list_split, axis=dim) | ||
if chunks == 1: | ||
return op.Identity(self) | ||
return op.Split(self, axis=dim, num_outputs=chunks) |
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 will need to fix the graph builder to return the correct number of outputs
No description provided.