NXP backend: Add recipes for Neutron backend lowering. - #21516
NXP backend: Add recipes for Neutron backend lowering.#21516MartinPavella wants to merge 2 commits into
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/21516
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 358c0d4 with merge base adaaa8b ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
The original implementation used an optional callback which if provided would override the logic in the QuantizeStage. This was done for simplicity. However, that approach made recipe fusing impossible and sort of went against the idea of recipes. Therefore, this commit introduces full QAT support into the QuantizeStage.
4c575c7 to
358c0d4
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds a recipe-driven export/lowering path for the NXP Neutron backend by extending the generic export pipeline to support additional lowering hooks (pre-partition callback, post-partition transforms) and a richer quantization recipe flow (PTQ/QAT phases + pass hooks). It also introduces NXP-specific recipe types/provider implementations and a new test suite to validate recipe behavior and combination.
Changes:
- Extend
EdgeTransformAndLowerStageto run an optionalpre_partitioning_callbackand applypost_partitioning_transformsfrom the lowering recipe. - Extend
QuantizeStage/QuantizationRecipeto support QAT (prepare_qat_pt2e), optional training, custom calibration inputs, and post-phase GraphModule pass hooks. - Add NXP Neutron recipe types/provider and a new test suite validating PTQ/QAT/no-delegate flows and recipe-combination behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| export/stages.py | Adds lowering pre/post partition hooks and expands quantization flow to support QAT/training/calibration hooks. |
| export/recipe.py | Extends recipe dataclasses (quantization + lowering) and updates recipe-combination logic for new hooks. |
| backends/nxp/tests/generic_tests/test_recipe_export.py | New tests covering Neutron recipe export behavior, config flags, and recipe combination. |
| backends/nxp/tests/executorch_pipeline.py | Deprecates imperative lowering helpers in favor of recipes. |
| backends/nxp/recipes/nxp_recipe_types.py | Introduces NXP-specific RecipeType enum for Neutron exports. |
| backends/nxp/recipes/nxp_recipe_provider.py | Implements the NXP recipe provider, recipe config, and lowering/quantization recipe builders. |
| backends/nxp/edge_passes/neutron_edge_pass_manager.py | Switches PassManager import to ExecuTorch’s pass manager implementation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( | ||
| hasattr(er := artifact.context["export_recipe"], "lowering_recipe") | ||
| and er.lowering_recipe is not None | ||
| and (callback := er.lowering_recipe.pre_partitioning_callback) is not None | ||
| ): | ||
| callback(self._partitioners, artifact.data) |
There was a problem hiding this comment.
Applicable, please use get method, similarily to line 272, or a shared local variable.
| def __call__(self, s: str, epm: EdgeProgramManager) -> NeutronEdgePassManager: | ||
| return self.neutron_edge_pass_manager |
There was a problem hiding this comment.
I think this is applicable (the Copilot's comment)
| input_spec: Iterable[ModelInputSpec] | tuple[int, ...] | list[tuple[int, ...]] | ||
| target: str = "imxrt700" | ||
| operators_not_to_delegate: list[str] = None | ||
| intermediates_dir: str | None = None |
There was a problem hiding this comment.
Applicable (the Copilot's comment).
| "NXP backend: The imperative lowering functions will be removed in the future. " | ||
| "Please use the recipe-based approach instead." | ||
| ) | ||
| def to_quantized_edge_program( |
There was a problem hiding this comment.
The recipes are still experimental:
export/recipe.py
@experimental(
"This API and all of its related functionality such as ExportSession and ExportRecipe are experimental."
)
@dataclass
class ExportRecipe:
I don't think we should deprecate our functions until recipes are production.
novak-vaclav
left a comment
There was a problem hiding this comment.
Below I have a few questions, comments and suggestions, otherwise very good job!!
I really like the idea of recipes, I believe it will make everything much more understandable for our users 👍👍👍😄
btw. I'm attaching an extended test suite my AI agents generated, feel free to run it to check the correctness after making modifications to the code or to draw inspiration from 😊
| input_spec: Model input description. Accepts a single shape tuple, a list of | ||
| shape tuples (one per input), or a list of ModelInputSpec objects. | ||
| target: Neutron hardware target string. Default: "imxrt700". | ||
| operators_not_to_delegate: Optional list of op names excluded from NPU delegation. |
There was a problem hiding this comment.
nit: I would add an example of entry in the operators_not_to_delegate list, like "aten::add".
| ) | ||
| from executorch.backends.nxp.recipes.nxp_recipe_types import NXP_BACKEND, NXPRecipeType | ||
| from executorch.backends.nxp.tests.executorch_pipeline import ( | ||
| _get_default_quantizer, |
There was a problem hiding this comment.
nit: From my coding experience in Python, methods with _ prefix are meant to be "private" and used only in the class/module they are defined or implemented in. Thus I think it would be better to rename the method in executorch_pipeline to get_default_quantizer without the prefix.
| compile_spec, | ||
| neutron_target_spec, | ||
| rc.custom_delegation_options, | ||
| preserve_ops=[torch.ops.aten.prelu.default], |
There was a problem hiding this comment.
This comment might not be directly related to this PR, but I think it's time to refactor the preserve_ops list we now use in multiple different calls (afaik it's here and in executorch_pipeline).
I'm saying this because the logic of preserving ops became more complicated and conditional in the aten.pad PR, and also Roman added aten.hardswish in his PR.
I suggest modifying NeutronPartitioner to set preserve_ops to [prelu, hardswish, pad] as default or extracting [prelu, hardswish, pad] to some global variable.
There is also a core_aten_ops_exception_list variable in our backend, which seems to do something similar to preserve_ops.
If you don't want to solve it in this PR, just add aten.pad and aten.harswish here and we will tackle the refactoring in another issue.
There was a problem hiding this comment.
It should be unified as I commented too. We have it also in aot_neutron_example.py.
| nodes = list(graph.nodes) | ||
|
|
||
| real_nodes = [n for n in nodes if n.op not in ("placeholder", "output")] | ||
| assert real_nodes[0].target != quantized_decomposed.quantize_per_tensor.out |
There was a problem hiding this comment.
nit: I think it would be more robust to check that the model input's child is the quant node and the model output's predecessor is dequant node.
There might be an edge case where there could be a model like this: x -> add(x, 2) -> y and the list of nodes could be ordered like this [x, get_attr(2), quant, add, dequant, y], since get_attr(2) and quant are on the same "level" in the model's graph.
Same goes for checking the dtype.
| assert placeholder_nodes[0].meta["val"].dtype == torch.int8 | ||
|
|
||
| def test_use_quant_state_dict_false(self): | ||
| """use_quant_state_dict=False: NeutronPartitioner gets None state_dict, no crash.""" |
There was a problem hiding this comment.
I'd check that post-quantization parameter values were not passed to the partitioner.
| input_spec: Iterable[ModelInputSpec] | tuple[int, ...] | list[tuple[int, ...]] | ||
| target: str = "imxrt700" | ||
| operators_not_to_delegate: list[str] = None | ||
| intermediates_dir: str | None = None |
There was a problem hiding this comment.
Applicable (the Copilot's comment).
| post_prepare_passes=[_post_prepare], | ||
| # Applied after training (or after prepare when no train_fn). | ||
| post_train_passes=[_remove_simulated_bn_and_fuse], | ||
| # Applied after calibration (only reached when train_fn is None). |
There was a problem hiding this comment.
Are you sure post_calibration_passes are applied only when train_fn is None?
in stages.py on line 512, I see post_calibration_passes are always run.
| m = move_exported_model_to_train(m) | ||
| qr.train_fn(m) | ||
|
|
||
| if qr.is_qat: |
There was a problem hiding this comment.
I would restructure these conditions containing qr.is_qat and qr.train_fn, since train_fn is not None implies qr.is_qat (or vice versa). Because of move_exported_model_to_train being idempotent, I would simply do:
if qr.train_fn is not None:
m = move_exported_model_to_train(m)
qr.train_fn(m)
m = move_exported_model_to_eval(m)
|
|
||
| input_spec: Iterable[ModelInputSpec] | tuple[int, ...] | list[tuple[int, ...]] | ||
| target: str = "imxrt700" | ||
| operators_not_to_delegate: list[str] = None |
There was a problem hiding this comment.
nit: the type of operators_not_to_delegate should be list[str] | None
| f"NXP backend: create_recipe() requires `{NEUTRON_RECIPE_CONFIG_KEY}=<NeutronRecipeConfig>`." | ||
| ) | ||
|
|
||
| if rc.custom_delegation_options is None: |
There was a problem hiding this comment.
I'm not a big fan of modifying the config that is passed to the recipe creator, I view it as an anti-pattern. Additionally, it might have unwanted consequences if someone decides to reuse the config somewhere else. Same goes for line 177, where the issue is even more severe.
If you decide to keep it anyway, please add a doc string to this method with a mention about this behavior.
roman-janik-nxp
left a comment
There was a problem hiding this comment.
Great job Martin 👍 . Implementation is solid in my opinion. I found mostly text errors or bugs in tests. I studied the code and the classes thoroughly to try to understand it.
| case NXPRecipeType.INT8_PTQ_NEUTRON: | ||
| return self._build_recipe(recipe_type, rc, is_qat=False, delegate=True) | ||
| case NXPRecipeType.INT8_QAT_NEUTRON: | ||
| if rc.train_fn is None: |
There was a problem hiding this comment.
Is the train_fn really needed to be required? In calibrate_and_quantize() we don't require it. In _build_quantization_recipe() you reference it "or after prepare when no train_fn".
| compile_spec, | ||
| neutron_target_spec, | ||
| rc.custom_delegation_options, | ||
| preserve_ops=[torch.ops.aten.prelu.default], |
There was a problem hiding this comment.
We should place the preserve_ops list in one place, I suggest a global variable in executor_pipeline.py.
| def is_cnn_op(n): | ||
| return any(op in n.name.lower() for op in ["conv", "relu", "view", "add"]) | ||
|
|
||
| assert not graph_contains_any(graph, is_cnn_op) |
There was a problem hiding this comment.
I wouldn't use ops names instead of targets like we do everywhere else. Applies everywhere.
| model = SimpleCNN() | ||
| rc = NeutronRecipeConfig(INPUT_SHAPE, get_quantizer_fn=my_quantizer_fn) | ||
| sess = _run_export(model, rc) | ||
| assert custom_quantizer_called, "Custom quantizer factory was not called." |
There was a problem hiding this comment.
| assert custom_quantizer_called, "Custom quantizer factory was not called." | |
| assert custom_quantizer_called, "Custom quantizer factory was called." |
| INPUT_SHAPE, get_calibration_inputs_fn=my_calibration_fn | ||
| ) | ||
| sess = _run_export(model, rc) | ||
| assert calibration_called, "Custom calibration function was not called." |
There was a problem hiding this comment.
| assert calibration_called, "Custom calibration function was not called." | |
| assert calibration_called, "Custom calibration function was called." |
| assert sess.get_edge_program_manager() is not None | ||
|
|
||
| def test_custom_delegation_options_explicit(self): | ||
| """Explicitly provided CustomDelegationOptions is forwarded correctly.""" |
There was a problem hiding this comment.
| """Explicitly provided CustomDelegationOptions is forwarded correctly.""" | |
| """Explicitly provided CustomDelegationOptions are forwarded correctly.""" |
|
|
||
| example_inputs = [(torch.randn(INPUT_SHAPE),)] | ||
| export(model, example_inputs=example_inputs, export_recipe=recipe) | ||
| assert transform_called, "post_partitioning_transforms were not executed." |
There was a problem hiding this comment.
| assert transform_called, "post_partitioning_transforms were not executed." | |
| assert transform_called, "post_partitioning_transforms were executed." |
| "NXP backend: The imperative lowering functions will be removed in the future. " | ||
| "Please use the recipe-based approach instead." | ||
| ) | ||
| def to_quantized_edge_program( |
There was a problem hiding this comment.
The recipes are still experimental:
export/recipe.py
@experimental(
"This API and all of its related functionality such as ExportSession and ExportRecipe are experimental."
)
@dataclass
class ExportRecipe:
I don't think we should deprecate our functions until recipes are production.
Summary
This PR introduces a declarative recipe-based lowering for the NXP Neutron backend. The previous solution was implemented as
executorch_pipeline.py:to_quantized_executorch_program(). The new solution provides the same functionailty and produces the same results. The benefit of the recipe-based approach is compatibility with other backends (recipe fusing) and adhering to ExecuTorch standards.Once this is merged, the old Neutron lowering pipeline can be removed completely.
Test plan
pytest backends/nxp/tests/generic_tests/test_recipe_export.pycc @robert-kalmar @JakeStevens @digantdesai @rascani