Skip to content
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

[CMSIS-NN] Aligned scale computation with TFLM to fix numerical mismatch #10817

Merged
merged 8 commits into from
Apr 6, 2022

Conversation

ashutosh-arm
Copy link
Contributor

This PR fixes numerical mismatch in Conv2D layers by
aligning order of output scale computation
with TFLM. Correct output scale is needed
to calculate quantization parameters needed
by CMSIS-NN.

Change-Id: I89bb99f46d2f04d4cd421c80cdbea2b7e34e9309
@ashutosh-arm
Copy link
Contributor Author

cc: @manupa-arm @Mousius @grant-arm for code review.

@@ -304,6 +304,94 @@ def test_conv2d_asymmetric_padding_int8(
)


# This test expects assertion as the output should mismatch between TVM and CMSIS-NN
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this assert that it matches the expected output? Similar to how the NPU tests check that they are consistent with TFLite?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we could compare against TFLite output. So far all the CMSIS-NN tests compare against TVM output.

Change-Id: I3b58d74a11e6c0649fe9454a85efd91cfbc52ccd
Change-Id: I5b788da3c34d9ed480d6f9197182e074df35e14e
Change-Id: I822ba3b99b47c3becdd925e1068b894ca07c84bb
…checks

Change-Id: I103b3b89692ac24249b619f0806e9f269f64dad3
Change-Id: Ibac370437e7961c1a65845d4861c3db2613f35ae
Change-Id: Icdcdc3bcc2f3330fc78410eede66c8c4ea273d43
Change-Id: I6f87d09b67c33b0cb37fef5d4d9fce72bd385cf8
@ashutosh-arm
Copy link
Contributor Author

@Mousius I have added a TFLite test. PTAL when you have time. As discussed offline, we can move this test to tvm/testing area in future.

Copy link
Member

@Mousius Mousius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the testing updates @ashutosh-arm 😸 looking forward to the generalisation later on!

@Mousius Mousius merged commit d7d8fdb into apache:main Apr 6, 2022
pfk-beta pushed a commit to pfk-beta/tvm that referenced this pull request Apr 11, 2022
…tch (apache#10817)

Fixes numerical mismatch in Conv2D layers by
aligning order of output scale computation
with TFLM. Correct output scale is needed
to calculate quantization parameters needed
by CMSIS-NN.
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request Apr 11, 2022
…tch (apache#10817)

Fixes numerical mismatch in Conv2D layers by
aligning order of output scale computation
with TFLM. Correct output scale is needed
to calculate quantization parameters needed
by CMSIS-NN.
Lucien0 pushed a commit to Lucien0/tvm that referenced this pull request Apr 19, 2022
…tch (apache#10817)

Fixes numerical mismatch in Conv2D layers by
aligning order of output scale computation
with TFLM. Correct output scale is needed
to calculate quantization parameters needed
by CMSIS-NN.
@ashutosh-arm ashutosh-arm deleted the numerical_mismatch branch April 22, 2022 08:35
altanh pushed a commit to altanh/tvm that referenced this pull request Apr 28, 2022
…tch (apache#10817)

Fixes numerical mismatch in Conv2D layers by
aligning order of output scale computation
with TFLM. Correct output scale is needed
to calculate quantization parameters needed
by CMSIS-NN.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants