NXP backend: Enable amin with new Neutron flow#20549
Conversation
|
| if is_alone_in_partition and keepdim and all(input_shape[d] == 1 for d in dim): | ||
| # The operator is a no-op, so the Neutron Converter will skip it. If it's the only node in the | ||
| # partition, the graph would end up empty. | ||
| return False |
There was a problem hiding this comment.
Well done for considering this case 👍🏻
Just FYI, NeutronPartitioner automatically tries to identify all no-ops by running every operator selected for delegation with random data. If it produces outputs that are exactly equal to the inputs (data, shape, type), it considers the node to be a no-op, and it will not delegate it as long as the operator is in a list of allowed no-ops. In the case of amin, I believe this system would have always correctly identified the no-ops, so simply adding the aten.amin into the list of allowed no-ops would have done the job.
But your solution is valid as well. No change needed.
There was a problem hiding this comment.
Thanks, I learn from the best (you 😄 ). However I don't see any logic for said identifying in NeutronPartitioner and when I omit these checks in supports_partitioning_result() the tests fail. So I'm keeping it.
| if not NodeConverter._has_shared_q_params_if_quantized(node): | ||
| return False |
There was a problem hiding this comment.
Where did you find this requirement?
There was a problem hiding this comment.
In your mean dim converter, I kept it, but I don't think it's necessary.
There was a problem hiding this comment.
It depends on the requirements of Neutron/Neutron IR. Often we can see requirements for equal input and output quantization here: https://developers.google.com/edge/litert/conversion/tensorflow/quantization/quantization_spec
The spec is, however, incomplete. It doesn't mention the ReduceMin operator. The Neutron documentation doesn't mention any such restrictions, so I think we should remove it. Also note that you used the shared quantization spec in our quantizer for the amin operator, so please update that too.
P.S.: We can handle mean later.
c780806 to
b7e0a71
Compare
b7e0a71 to
dd1f281
Compare
dd1f281 to
368a3f2
Compare
Summary
Add tests verifying correct support for amin by the Neutron backend using the new Neutron MLIR flow.
Test plan
Unit tests provided.
cc @robert-kalmar