NXP backend: Enable adaptive_avg_pool2d with new Neutron flow.#19540
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19540
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: ❌ 2 New Failures, 3 Unrelated FailuresAs of commit 6a6b94c with merge base b04cc65 ( NEW FAILURES - The following jobs have failed:
FLAKY - The following job failed but was likely due to flakiness present on trunk:
BROKEN TRUNK - The following jobs 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. |
|
@novak-vaclav as per usual, cannot add you as a reviewer but please feel free to have a look :) |
30aaa4e to
4fed4de
Compare
| parameters_mapping: dict[str, Parameter], | ||
| custom_delegation_options: CustomDelegationOptions, | ||
| ) -> bool: | ||
| if ( |
There was a problem hiding this comment.
I would move this check to is_supported_on_target, since it's not an IR's problem we did not determine node format yet.
It also makes more sense because _get_equivalent_avg_pool_parameters is also used in is_supported_on_target and the code concerning node formats would be in one place.
There was a problem hiding this comment.
The check is there to make sure our NodeFormatInference handles the operator as we expected. It is there to make sure that the principles on which EdgeProgramToIRConverter is built are obeyed. It has nothing to do with the RT700 platform and it's Neutron parameters. It is strictly related to conversion to the IR. Therefore it doesn't make sense for it to be in is_supported_on_target. I agree that including it in is_supported_in_IR is not ideal (though it's not a big stretch).
The PR which originally added the adaptive avgpool2d to our backend didn't update the format inference, so I added this check. I believe we should add it to all node converters of operators which require channels last format in NeutronIR.
Let's leave it as is in this PR. I have created a ticket to address this: https://jira.sw.nxp.com/browse/EIEX-913
I think I'll get on it soon.
|
Nice work! 👍 I have only a few points for improvement |
4fed4de to
6a6b94c
Compare
Summary
This PR reflects the new Neutron requirements for the enablement of
adaptive_avg_pool_2din NXP backend,Test plan
Unit tests provided.
cc @robert-kalmar @JakeStevens @digantdesai @rascani