Skip to content

NXP backend: Add support for softmax with the new Neutron flow. - #19739

Draft
irtrukhina wants to merge 2 commits into
pytorch:mainfrom
nxp-upstream:feature/EIEX-728_revisit_softmax_support
Draft

NXP backend: Add support for softmax with the new Neutron flow.#19739
irtrukhina wants to merge 2 commits into
pytorch:mainfrom
nxp-upstream:feature/EIEX-728_revisit_softmax_support

Conversation

@irtrukhina

@irtrukhina irtrukhina commented May 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

Add support for the softmax to the Neutron backend using the new Neutron MLIR flow. Add new tests and update old tests for the softmax as well.

Test plan

New tests are included in the PR.

cc @robert-kalmar @JakeStevens @digantdesai @rascani

@irtrukhina
irtrukhina requested a review from robert-kalmar as a code owner May 22, 2026 08:08
@pytorch-bot

pytorch-bot Bot commented May 22, 2026

Copy link
Copy Markdown

🔗 Helpful Links

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

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

✅ No Failures

As of commit 79b5ba5 with merge base d29706a (image):
💚 Looks good so far! There are no failures yet. 💚

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 May 22, 2026
@linux-foundation-easycla

linux-foundation-easycla Bot commented May 22, 2026

Copy link
Copy Markdown

CLA Not Signed

@robert-kalmar robert-kalmar 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 May 22, 2026
Comment thread backends/nxp/tests/generic_tests/test_integration.py Outdated
Comment thread backends/nxp/tests/ir/converter/node_converter/test_softmax_converter.py Outdated
Comment thread backends/nxp/tests/ir/edge_passes/test_remove_io_quant_ops_pass.py Outdated
@irtrukhina
irtrukhina force-pushed the feature/EIEX-728_revisit_softmax_support branch from 154ffff to 56a1296 Compare June 1, 2026 16:09
Comment thread backends/nxp/tests/ir/converter/node_converter/test_softmax_converter.py Outdated
@irtrukhina
irtrukhina force-pushed the feature/EIEX-728_revisit_softmax_support branch from c8324ed to c122421 Compare June 4, 2026 08:54
Comment thread backends/nxp/tests/ir/converter/node_converter/test_softmax_converter.py Outdated
1. Input and Output must be INT8/UINT8
2. Channels <= 2040
3. Total spatial size (H*W) <= 4096
3. Total spatial size <= 4096

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: As far as I know total spatial size is not a universally known term. Before your change, it was supposed to represent H*W, whereas now it means N*H*W. Please keep the explicit definition here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Kept the explicit definition (NHW)

@MartinPavella

Copy link
Copy Markdown
Collaborator

Please take a look at the canceled pull / unittest-nxp-neutron / linux-job (pull_request). Otherwise, LGTM

@irtrukhina
irtrukhina force-pushed the feature/EIEX-728_revisit_softmax_support branch from 502f045 to f504223 Compare June 9, 2026 08:09
@robert-kalmar
robert-kalmar marked this pull request as draft June 23, 2026 16:50
@irtrukhina
irtrukhina force-pushed the feature/EIEX-728_revisit_softmax_support branch from f504223 to 8cc126d Compare July 14, 2026 14:23
@irtrukhina
irtrukhina force-pushed the feature/EIEX-728_revisit_softmax_support branch from 8cc126d to 0d13bba Compare July 31, 2026 10:26
@irtrukhina

Copy link
Copy Markdown
Collaborator Author

@MartinPavella, could you please take a final look at the PR? All tests have passed, and the documentation has been fixed to meet our checks.

@MartinPavella MartinPavella left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just a couple of observations to consider. No blockers.

Comment on lines +56 to +58
# Allow MSE up to the theoretical error introduced by 1-bit quantization (1/256).
comparator = NumericalStatsOutputComparator(
max_mse_error=1.0e-3, is_classification_task=True
max_mse_error=0.00390625,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't see how the dequantized value of a single-bit error directly dictates the MSE threshold. So the comment doesn't make sense to me. (same for the following test)

Also Nit: Using 1/256 would be cleaner.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Sorry. This issue was caused by a combination of my brain overheating in the summer heat and a copy-paste error. The MSE should be the squared one-bit quantization error. I fixed the comparator settings and removed the comment.

expected_delegated_ops={Softmax: 1},
expected_non_delegated_ops={},
)
output_comparator = NumericalStatsOutputComparator(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why are you using the NumericalStatsOutputComparator?
For other ops we use the AllCloseOutputComparator with a quantized dataset and atol=1. So it is clear only single-bit errors are tollerated.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It is a very old PR and was created before we add remove_quant_io_ops option into lower_run_compare.
Replaced NumericalStatsOutputComparator with AllCloseOutputComparator(atol=1).

@roman-janik-nxp roman-janik-nxp left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I recommend to fix the issues mentioned by Martin.

],
)
def test_softmax_delegation__channel_first(input_shape, dim: int, mocker):
model = ConvSoftmaxModule(dim, input_shape[1])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there are reason why no longer test channel first variant?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It doesn't make sense to me. Neutron only supports channels-last format. Any errors in the channels-first version are more likely caused by incorrect dimension ordering than by the softmax operation.
These tests are specifically focused on validating softmax.
Do you agree?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Well spotted @roman-janik-nxp, we should definitely have a test with a softmax after a convolution/maxpool/... node to make sure our SoftmaxConverter handles that edge case correctly.
@irtrukhina please add such a test.

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.

4 participants