-
Notifications
You must be signed in to change notification settings - Fork 685
NXP backend: Add infrastructure for context dependant partitioning #14373
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
NXP backend: Add infrastructure for context dependant partitioning #14373
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/14373
Note: Links to docs will display an error until the docs builds have been completed. ❌ 4 New Failures, 2 Cancelled Jobs, 2 Unrelated FailuresAs of commit 8ad83cf with merge base b3f3111 ( NEW FAILURES - The following jobs have failed:
CANCELLED JOBS - The following jobs were cancelled. Please retry:
FLAKY - The following job failed but was likely due to flakiness present on trunk:
BROKEN TRUNK - The following job failed but was present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
@pytorchbot label "module: nxp" "release notes: nxp" |
123e92d
to
1b7ae0e
Compare
2cf03eb
to
5fcf3c6
Compare
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.
Now that the unit tests restore the original state of supports_partitioning_result, the PR looks OK.
6325471
to
cdadf15
Compare
The failing test |
filter(is_not_qdq_node, view_copy_partitions[0].nodes) | ||
) | ||
|
||
if len(non_q_dq_partition_nodes) == 1: |
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.
For any practical model the contion is enough, but in theory not right.
Thinking loudly:
- The view_copy (==> Reshape in Neutron IR) cannot be alone in the partition because on Neutron it is a NOOP. Regardless of the count (2 view_copies is also not allowed)
- Another operator is clone, as we convert it to an "identity" tensor, what will be NOOP in neutron too.
So I would say, the correct condition is there must be at least 1 compute operator, that is the partition must not contain only view_copy and clone ops (any number).
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.
Good catch.
I will create a ticket for this, as it is not trivial to determine which nodes will end up being "noops". And it would require another PR to be merged first.
cdadf15
to
8ad83cf
Compare
Summary
This PR adds the option to specify delegation conditions which depend on the partition a particular node ends up in. This infrastructure is applied to the
view_copy
node.Test plan
Unit tests provided.
cc @robert-kalmar @roman-janik-nxp @StrycekSimon @jirioc