Skip to content

Conversation

MartinPavella
Copy link
Collaborator

@MartinPavella MartinPavella commented Aug 28, 2025

Summary

This PR introduces an aten dialect pre-processing pass which splits conv nodes with group>1 into multiple parallel conv nodes with group=1. This replaces the original implementation in Neutron IR.

Test plan

Unit tests provided in backends/nxp/tests/test_split_group_convolution.py

cc @robert-kalmar @roman-janik-nxp @StrycekSimon @jirioc

Copy link

pytorch-bot bot commented Aug 28, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/13758

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

❌ 2 New Failures

As of commit fe37a89 with merge base 1520f9f (image):

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 28, 2025
@MartinPavella
Copy link
Collaborator Author

@pytorchbot label "module: nxp" "release notes: nxp"

@pytorch-bot pytorch-bot bot added module: nxp Issues related to NXP Neutron NPU delegation and code under backends/nxp/ release notes: nxp Changes to the NXP Neutron backend delegate labels Aug 28, 2025
@MartinPavella MartinPavella force-pushed the upstream/main-nxp/EIEX-472-separable-convolution-decomposition-in-executorch branch from 647412a to f016892 Compare August 28, 2025 12:51
@MartinPavella MartinPavella force-pushed the upstream/main-nxp/EIEX-472-separable-convolution-decomposition-in-executorch branch from f016892 to 2e65f1d Compare August 28, 2025 13:58
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you remove the tests for the separable convolution and not update them? Now they can convert to Neutron, or we miss some Ops?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8 tests were removed in total:

  • 4 of them tested the group conv decomposition with a float32 model. These were used to test the IR decomposition implementation. They were not that useful as they did not test the intended use-case (quantized model).
  • The last 4 used a quantized model, but as split is not yet supported, the group conv couldn't be delegated. So these tests only verified that the node was not delegated.

8 new tests were added in backends/nxp/tests/test_split_group_convolution.py, which test the decomposition and the delegation together.

Updating the 4 removed floating point tests would result in basically duplicate tests. And as the decomposition is no longer the responsibility of the ConvConverter, it made more sense to me to test the new functionality in a new module.

@robert-kalmar
Copy link
Collaborator

  • Fix the tests. The unit tests are failing.

@MartinPavella MartinPavella force-pushed the upstream/main-nxp/EIEX-472-separable-convolution-decomposition-in-executorch branch 2 times, most recently from d4b7aa3 to fb20072 Compare September 1, 2025 07:18
@MartinPavella MartinPavella force-pushed the upstream/main-nxp/EIEX-472-separable-convolution-decomposition-in-executorch branch from fb20072 to fe37a89 Compare September 1, 2025 08:08
@robert-kalmar robert-kalmar merged commit e254f0e into pytorch:main Sep 2, 2025
111 of 113 checks passed
@robert-kalmar robert-kalmar deleted the upstream/main-nxp/EIEX-472-separable-convolution-decomposition-in-executorch branch September 2, 2025 07:07
@shoumikhin
Copy link
Contributor

@MartinPavella executorch/backends/nxp/tests:test_quantizer test failed for us internally on this PR, can you please check if this looks related?

ImportError while importing test module '/data/sandcastle/boxes/trunk-hg-full-fbsource/buck-out/v2/gen/fbcode/a9f8037888495e78/executorch/backends/nxp/tests/__test_quantizer__/test_quantizer#link-tree/executorch/backends/nxp/tests/test_quantizer.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/local/fbcode/platform010/lib/python3.12/importlib/__init__.py:135: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
../buck-out/v2/gen/fbcode/a9f8037888495e78/executorch/backends/nxp/tests/__test_quantizer__/test_quantizer#link-tree/executorch/backends/nxp/tests/test_quantizer.py:12: in <module>
    from executorch.backends.nxp.quantizer.neutron_quantizer import NeutronQuantizer
../buck-out/v2/gen/fbcode/a9f8037888495e78/executorch/backends/nxp/tests/__test_quantizer__/test_quantizer#link-tree/executorch/backends/nxp/quantizer/neutron_quantizer.py:11: in <module>
    from executorch.backends.nxp.aten_passes.neutron_aten_pass_manager import (
../buck-out/v2/gen/fbcode/a9f8037888495e78/executorch/backends/nxp/tests/__test_quantizer__/test_quantizer#link-tree/executorch/backends/nxp/aten_passes/neutron_aten_pass_manager.py:16: in <module>
    from executorch.backends.nxp.aten_passes.split_group_convolution import (
../buck-out/v2/gen/fbcode/a9f8037888495e78/executorch/backends/nxp/tests/__test_quantizer__/test_quantizer#link-tree/executorch/backends/nxp/aten_passes/split_group_convolution.py:10: in <module>
    from executorch.backends.nxp.backend.ir.converter.node_converters.shared.conv_utils import (
E   ModuleNotFoundError: No module named 'executorch.backends.nxp.backend'
=============================== warnings summary ===============================
../buck-out/v2/gen/fbcode/a9f8037888495e78/executorch/backends/nxp/tests/__test_quantizer__/test_quantizer#link-tree/_pytest/assertion/rewrite.py:934: 52 warnings
  /data/sandcastle/boxes/trunk-hg-full-fbsource/buck-out/v2/gen/fbcode/a9f8037888495e78/executorch/backends/nxp/tests/__test_quantizer__/test_quantizer#link-tree/_pytest/assertion/rewrite.py:934: DeprecationWarning: ast.NameConstant is deprecated and will be removed in Python 3.14; use ast.Constant instead
    clear = ast.Assign(variables, ast.NameConstant(None))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. module: nxp Issues related to NXP Neutron NPU delegation and code under backends/nxp/ release notes: nxp Changes to the NXP Neutron backend delegate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants