[CMSIS-NN] Fixed the case with repeating operands in the QNN binary ops #11732
Conversation
|
cc: @Mousius @grant-arm @manupa-arm for code review. |
There was a problem hiding this comment.
I have a few questions @ashutosh-arm, hopefully I've not missed the obvious 😸
| "input": np.random.randint(in_min, high=in_max, size=shape, dtype=dtype), | ||
| } | ||
| output_list = generate_ref_data(orig_mod["main"], inputs) | ||
| compile_and_run( |
There was a problem hiding this comment.
Should we not test that the internal functions only have 1 parameter each?
There was a problem hiding this comment.
I have added a check for this above in L182. Thanks @Mousius.
tests/python/contrib/test_cmsisnn/test_scalar_to_tensor_constant.py
Outdated
Show resolved
Hide resolved
| mod = relay.transform.InferType()(mod) | ||
| mod = ScalarToTensorConstants()(mod) | ||
| new_mod = relay.transform.InferType()(mod) | ||
| assert tvm.ir.structural_equal(mod[global_var].body, new_mod[global_var].body) |
There was a problem hiding this comment.
What is this checking? It appears to just check the body hasn't changed after InferType?
There was a problem hiding this comment.
Before this commit, it was producing a different body in the end due to an error in the code. Since the relay model in this test does not contain any scalar constants, expectation is that the pass should not affect the graph.
There was a problem hiding this comment.
From offline discussion it was decided to leave the check for structural equality in there.
…omposite function Change-Id: I7ffd6074bbbe9020b6efe64d48b80f79714ce8bd
ashutosh-arm
force-pushed
the
yolo_cmsisnn_1
branch
from
121db50 to
f984e0e
Compare
|
Thanks @Mousius for the discussions and code review. Would you like to take a look at it again? |
|
Thanks @ashutosh-arm 😸 |
This commit is to fix issues in CMSIS-NN passes
that surface when the same operand is repeated
in QNN binary ops.
cc @areusch