NXP backend: Test max_pool2d with new Neutron flow.#19272
NXP backend: Test max_pool2d with new Neutron flow.#19272MartinPavella wants to merge 2 commits intopytorch:mainfrom
max_pool2d with new Neutron flow.#19272Conversation
MartinPavella
added
the
module: nxp
MartinPavella
added
the
release notes: nxp
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19272
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ✅ You can merge normally! (2 Unrelated Failures)As of commit 7096412 with merge base 48a8d58 ( BROKEN TRUNK - The following jobs failed but were 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. |
meta-cla
Bot
added
the
CLA Signed
|
@novak-vaclav I am not able to add you as a reviewer, but feel free to have a look. |
This comment was marked as resolved.
This comment was marked as resolved.
|
|
||
| class TestMaxPool2DNewNeutronFlow: | ||
| # noinspection PyMethodMayBeStatic | ||
| def assert_delegated(self, model, input_shape): |
There was a problem hiding this comment.
I would move these methods to a separate file so they can be used in other tests as well. When I start implementing new neutron flow tests, I would have to copy this into my test suite and that seems unnecessary.
There was a problem hiding this comment.
These functions are not universal. This one checks for 1 delegated node and no non-delegated nodes. For example MaxPool1D tests have it different.
I guess we could implement some generic methods like this, but let's not do it in this PR.
| def test__basic_nsys_inference__view_not_delegated(self): | ||
| input_shape = (2, 4, 6) # The old flow limited the batch size to 1. | ||
| model = MaxPool1DModule() | ||
| graph_verifier = BaseGraphVerifier( |
There was a problem hiding this comment.
I would add checking for the maxpool_2d node explicitly. Is it possible to tell if the delegated node was originally maxpool_2d?
There was a problem hiding this comment.
Not with the BaseGraphVerifier. If there is only 1 node in the model (like in this case), there is no doubt it is the MaxPool. But I like the idea of a more strict GraphVerifier. I have created a task for it: https://jira.sw.nxp.com/browse/EIEX-902
MartinPavella
force-pushed
the
nxg01483/EIEX-868-add-max_pool2d-support-using-new-neutron-flow
branch
from
a3f359a to
7096412
Compare
novak-vaclav
left a comment
novak-vaclav
left a comment
There was a problem hiding this comment.
LGTM. Good work 👍
(for some reason I don't see the "Resolve comment" in the Github GUI and resolving the comments in the diff view throws "Failed to resolve thread" 🤷)
2 participants
Summary
Reflect the requirements of the new Neutron MLIR flow for the
max_pool2doperator in NXP backend.Test plan
Unit tests provided.
cc @robert-kalmar @JakeStevens @digantdesai