-
Notifications
You must be signed in to change notification settings - Fork 508
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
Arm backend: Add WhyNoPartitionReporter and report rejected nodes #8963
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/8963
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 Cancelled Job, 1 PendingAs of commit 802d148 with merge base 5cd973c ( CANCELLED JOB - The following job was cancelled. Please retry:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
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.
In general this looks good to me, and is what i'm hoping our partitioner logging infra can look like moving forward.
Would love to see placeholder tensors reported in the future, but that can be added later i suppose.
28f0d98
to
5a5dc87
Compare
With more complex checks for if nodes are supported or not, it can probably be complicated for a user to understand why a ceratin node was rejected from partitioning. The WhyNoPartitionReporter attempts to mitigate this by producing a table with all rejected nodes with a reason for why they were rejected. The reasoning behind using a class rather than log statements is - The table keeps all reject information in the same place. - Uniform formatting. - Only log the first reject report of a node. - Easier to change how and when the output is logged/dumped to file. Signed-off-by: Erik Lundell <[email protected]> Change-Id: I3765a0db4bfe530b34a7c5d48b04faa5d08e51de
5a5dc87
to
f3e823e
Compare
MacOs fails unrelatred |
…torch#8963) With more complex checks for if nodes are supported or not, it can probably be complicated for a user to understand why a ceratin node was rejected from partitioning. The WhyNoPartitionReporter attempts to mitigate this by producing a table with all rejected nodes with a reason for why they were rejected. The reasoning behind using a class rather than log statements is - The table keeps all reject information in the same place. - Uniform formatting. - Only log the first reject report of a node. - Easier to change how and when the output is logged/dumped to file. Signed-off-by: Erik Lundell <[email protected]>
…torch#8963) With more complex checks for if nodes are supported or not, it can probably be complicated for a user to understand why a ceratin node was rejected from partitioning. The WhyNoPartitionReporter attempts to mitigate this by producing a table with all rejected nodes with a reason for why they were rejected. The reasoning behind using a class rather than log statements is - The table keeps all reject information in the same place. - Uniform formatting. - Only log the first reject report of a node. - Easier to change how and when the output is logged/dumped to file. Signed-off-by: Erik Lundell <[email protected]>
With more complex checks for if nodes are supported or not, it can probably be complicated for a user to understand why a ceratin node was rejected from partitioning.
The WhyNoPartitionReporter attempts to mitigate this by producing a table with all rejected nodes with a reason for why they were rejected.
The reasoning behind using a class rather than log statements is
cc @digantdesai @freddan80 @per @zingo @oscarandersson8218