-
Notifications
You must be signed in to change notification settings - Fork 600
NXP backend: Add support for depthwise and separable convolution. #11215
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?
NXP backend: Add support for depthwise and separable convolution. #11215
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/11215
Note: Links to docs will display an error until the docs builds have been completed. ❌ 3 New FailuresAs of commit 07f79c2 with merge base 77f16dc ( NEW FAILURES - The following jobs have failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
@pytorchbot label "module: nxp" "release notes: nxp" |
@@ -204,3 +210,328 @@ def test_conv2d_quant_conversion(mocker, model: torch.nn.Module, input_shape): | |||
input_data=input_data, | |||
atol=1.0, | |||
) | |||
|
|||
|
|||
@pytest.mark.parametrize("stride", [1, 2]) |
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 thought we decided to not use pytest features in new tests :P
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.
Sorry for misunderstanding. I thought we are OK using pytest temporarily for the those tests, we already have in our development tree.
The latest (in our development tree) uses the unittest
instead of pytest
- nxp-upstream@63fd43f#diff-d103a8083dbc6da9184c269a015199baa963f734cdd8cb2f29336e6954f01cb8R7
assert len(nodes) == 11 | ||
assert ( | ||
nodes[7].target.__name__ == "aten.convolution.default" | ||
) # Convolution not delegated. |
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.
Love these tests.
Just skimmed, let me take a close look in a day or so, esp |
The `group` attribute of the `aten.convolution` has an effect on how the weights are used in the computation of the convolution. To properly utilize Neutron, the convolution is sometimes converted to `DepthwiseConv2D`.
8e64288
to
cb91593
Compare
Ready for review. Most of the tests passing. The failing seems unrelated to the changes:
|
digant is out for a while I will try to find another person to review these |
Neutron only supports batch == 1.
Converting to draft unless the NXP Backend CI is back (#11756) |
I am back, I can review this coming week. |
Summary
Adding depthwise convolution support for the NXP backend
Test plan
Functionality tested by python unit tests:
cc @digantdesai @JakeStevens , @Pop-korn