-
Notifications
You must be signed in to change notification settings - Fork 664
NXP backend: Catching the converter failures. #14117
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
NXP backend: Catching the converter failures. #14117
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/14117
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (6 Unrelated Failures)As of commit c3f5a69 with merge base 6ed10e5 ( 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. |
To add the ciflow label This helps ensure we don't trigger CI on this PR until it is actually authorized to do so. Please ping one of the reviewers if you do not have access to approve and run workflows. |
@@ -3,11 +3,18 @@ | |||
# This source code is licensed under the BSD-style license found in the |
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.
# Copyright 2024-2025 NXP
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.
Fixed ✅
@@ -52,6 +59,25 @@ def convert( | |||
cctx.targetOpts = neutron_converter.getNeutronTarget(target) | |||
# New switch since Neutron Converter SDK_25.06 | |||
cctx.compilationOpts.minNumOpsPerGraph = 1 | |||
model_converted = neutron_converter.convertModel(list(tflite_model), cctx) | |||
# Don't merge `Transpose` operators, to avoid creating unsupported permutations. | |||
cctx.compilationOpts.excludeGraphPasses = "MergeTranspose" |
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.
This should probably not be included, as proper permute_copy
support has not been up-streamed yet.
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 removed it. ✅
import pkgutil | ||
|
||
from executorch.backends.nxp.backend.ir.converter.node_converter import Target | ||
|
||
|
||
def convert_unsafe(neutron_converter, tflite_model, cctx, queue): |
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.
A documentation comment would be useful here.
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.
@jirioc, can you please add it?
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.
Done
62ab20c
to
fce6017
Compare
fce6017
to
ee901ae
Compare
ee901ae
to
c3f5a69
Compare
Seems like the failed tests are unrelated to this PR. |
Summary
This feature makes the flow robust against Neutron converter crash/exit. Especially useful for running unit tests.
Test plan
When the nxp unit tests pass, the Neutron converter is properly invoked. No need for extra testing.
cc @digantdesai @JakeStevens @robert-kalmar