NXP backend: added support for scalar args of elementary ops#21011
NXP backend: added support for scalar args of elementary ops#21011novak-vaclav wants to merge 1 commit into
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/21011
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New FailureAs of commit f7efe57 with merge base 43bdb7b ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
MartinPavella
left a comment
There was a problem hiding this comment.
Just a few suggestions. Very nice work 👍🏻
| if isinstance(node_val, torch.Tensor): | ||
| return node_val.dtype | ||
|
|
||
| return torch.float32 |
There was a problem hiding this comment.
Nit:
Perhaps it would be better to call off the modification if we cannot infer the type.
|
|
||
| return torch.float32 | ||
|
|
||
| def _create_scalar_attr_node(self, node: Node, scalar_value: int | float) -> Node: |
There was a problem hiding this comment.
Nit:
Type hint int | float is equivalent to just float.
| exir_program_aten = torch.export.export(model, (example_input,)).module() | ||
|
|
||
| # Check if the node with scalar arg did not disappear. | ||
| assert graph_contains_any_of_ops(exir_program_aten.graph, [expected_op]) |
There was a problem hiding this comment.
Nit:
How about a check here to verify that the op indeed has a scalar argument?
|
Hmm, I just came across this: https://github.com/pytorch/executorch/blob/main/backends/transforms/replace_scalar_with_tensor.py |
Summary
Added support for scalar args of elementary ops.
Test plan
tests can be manually run using
pytest -c /dev/null backends/nxp/tests/cc @robert-kalmar @JakeStevens @digantdesai @rascani @MartinPavella