[microNPU] Update Conv2D Tests to Use TF API to Gen Test Cases by dchauhan-arm · Pull Request #9508 · apache/tvm

dchauhan-arm · 2021-11-15T11:11:41Z

Previous implementation of conv2d tests would compare the conv2d operator against tvm's execution of the default schedule of conv2d as defined in TOPI, and that is not bitexact with tflite runtime's implemention. Therefore, a tolerance of "1" in quantized 8-bit domain was used.
Converts the current conv2d tests to use TensorFlow APIs to create test cases for conv2D, and compare against TFLite runtime.

Change-Id: I74b4a2a22fe90fa0d9176a65627adf81f127dbd0

dchauhan-arm · 2021-11-15T11:12:50Z

cc @manupa-arm @ekalda @lhutton1 @NicolaLancellotti @mbaret

manupak

Thanks for the cleaning this up @dchauhan-arm ! Much appreciated :).
Broadly looks good and few suggestions to improve it a bit.

ekalda

Thanks for cleaning this up, good first patch :) While you are at it, can you also remove relay_ir_builder.py file since after this change we don't use relay_ir_builder any more?

manupak · 2021-11-23T10:37:25Z

@dchauhan-arm Shall we try to get this merged?

lhutton1

Thanks @dchauhan-arm it looks like this PR is almost ready :) Could you fix the formatting issues and a couple of very minor things?

It looks like linting is failing on running black, you can use ./docker/bash.sh tvm.ci_lint ./tests/lint/git-black.sh -i HEAD~1 locally to fix this

ekalda

Mostly looks good (modulo Luke's comments and linting fixes)! :)

ekalda · 2021-11-25T17:07:44Z

+                op = tf.nn.conv2d(
+                    x,
+                    filters=tf.constant(np.random.uniform(
+                    	size=(kernel_shape[0], kernel_shape[1], 3, 3)), dtype=tf.float32),
+                    strides=strides
+                    padding=padding,
+                    data_format="NHWC",
+                    dilations=dilation,
+                )


This currently tests a special case of input channels and output channels being equal, I think it would be better to test a more general case for the conv2d (e.g. if the legalization pass got ifm and ofm channels mixed up, we would never find out), but I wonder what @manupa-arm thinks about whether it's worth changing?

Its a good point -- lets follow up with another PR to improve coverage.

Update ordering of imports

Remove unused import

ekalda

Almost there :) It needs to be rebased on top of #9597, but otherwise mostly looks good!

Address comments, use infra.py function to compute ofm_shape

Linting

Missing square brackets in parametrization, missing underscores.

CI- re trigger

Fix imports

ekalda

LGTM, let's get this in! :)

lhutton1

LGTM! Well done with battling the multiple rebases this needed :)

manupak

LGTM!.

Thanks for the hard work! and congratulations on getting your first PR in 🎉 .
I think we need to follow up to use a similiar structure as other tests (after @mbaret's refactor).
to be consistent.

manupak · 2021-12-09T13:29:19Z

Thanks @dchauhan-arm @lhutton1 @ekalda @NicolaLancellotti!

…e#9508) * Current conv2d tests compare the conv2d operator against tvm's execution of the default schedule of conv2d as defined in TOPI and that is not bitexact with tflite runtime's implemention. Therefore a tolerance of "1" in quantized 8-bit domain is used. * Converts the current conv2d tests to use TensorFlow APIs to create a test cases for conv2D and compare against TFLite runtime.

In apache#9508 the decision was made to remove the UnsupportedLayout exception and the checks that throw it, this PR is cleaning up some that remained. Change-Id: I83bfe233381b83af886343c9569db753e33f9059

…e#9508) * Current conv2d tests compare the conv2d operator against tvm's execution of the default schedule of conv2d as defined in TOPI and that is not bitexact with tflite runtime's implemention. Therefore a tolerance of "1" in quantized 8-bit domain is used. * Converts the current conv2d tests to use TensorFlow APIs to create a test cases for conv2D and compare against TFLite runtime.

* [microNPU] Remove remaining UnsupportedLayout checks In #9508 the decision was made to remove the UnsupportedLayout exception and the checks that throw it, this PR is cleaning up some that remained. Change-Id: I83bfe233381b83af886343c9569db753e33f9059 * fix lint Change-Id: I67c1a5371f0b2e51b6cd39435ef4073d8d17af51

manupak

Summarizing the offline conversation we had to w.r.t. removal of the custom exception.

manupak · 2022-01-17T14:50:50Z

-"""This module defines all error types associated with the Arm(R) Ethos(TM)-U NPU code generator."""
-
-
-class EthosUCodegenError(Exception):


We decided to remove the layout checks as we decided to rely on partitioning not to partition unsupported layouts to the codegen. Therefore, they are not likely to be exercised from a user flow. Therefore should not be an exception.

Furthermore, it felt unlikely to be triggered in dev flow too to replace them with an assertion -- however, we could revisit if that is experienced frequently.

* [microNPU] Remove remaining UnsupportedLayout checks In apache#9508 the decision was made to remove the UnsupportedLayout exception and the checks that throw it, this PR is cleaning up some that remained. Change-Id: I83bfe233381b83af886343c9569db753e33f9059 * fix lint Change-Id: I67c1a5371f0b2e51b6cd39435ef4073d8d17af51

…e#9508) * Current conv2d tests compare the conv2d operator against tvm's execution of the default schedule of conv2d as defined in TOPI and that is not bitexact with tflite runtime's implemention. Therefore a tolerance of "1" in quantized 8-bit domain is used. * Converts the current conv2d tests to use TensorFlow APIs to create a test cases for conv2D and compare against TFLite runtime.

dchauhan-arm requested review from areusch, comaniac, icemelon, jroesch, junrushao, merrymercy, tqchen and yzhliu as code owners November 15, 2021 11:11

manupak requested changes Nov 15, 2021

View reviewed changes

Comment thread tests/python/contrib/test_ethosu/test_codegen.py Outdated

Comment thread tests/python/contrib/test_ethosu/test_legalize.py Outdated

Comment thread tests/python/contrib/test_ethosu/test_legalize.py

Comment thread tests/python/contrib/test_ethosu/test_codegen.py Outdated

ekalda requested changes Nov 15, 2021

View reviewed changes

ekalda mentioned this pull request Nov 16, 2021

[Tracking Issue] Arm(R) Ethos(TM)-U NPU operator support #9410

Closed

15 tasks

lhutton1 mentioned this pull request Nov 22, 2021

[microNPU] Add unary elementwise operator infrastructure with ABS #9530

Merged

dchauhan-arm requested review from MarisaKirisame, ZihengJiang, anijain2305, jwfromm, mbrookhart, slyubomirsky, vinx13, wweic and zhiics as code owners November 23, 2021 14:56

dchauhan-arm requested a review from manupak November 25, 2021 09:48

lhutton1 reviewed Nov 25, 2021

View reviewed changes

Comment thread tests/python/contrib/test_ethosu/test_codegen.py Outdated

Comment thread tests/python/contrib/test_ethosu/test_legalize.py Outdated

ekalda reviewed Nov 25, 2021

View reviewed changes

dchauhan-arm requested review from Huyuwei, Hzfengsy, jcf94 and kazum as code owners November 29, 2021 12:18

dchauhan-arm added 2 commits December 1, 2021 12:59

[microNPU] Update Conv2D Tests to Use TF API to Gen Test Cases

9e69c19

Update ordering of imports

[microNPU] Update Conv2D Tests to Use TF API to Gen Test Cases

8d1894e

Remove unused import

ekalda reviewed Dec 2, 2021

View reviewed changes

Comment thread tests/python/contrib/test_ethosu/test_legalize.py Outdated

Comment thread tests/python/contrib/test_ethosu/test_legalize.py Outdated

dchauhan-arm and others added 6 commits December 2, 2021 14:55

[microNPU] Update Conv2D Tests to Use TF API to Gen Test Cases`

9bd23d5

Address comments, use infra.py function to compute ofm_shape

[microNPU] Update Conv2D Tests to Use TF API to Gen Test Cases

5e23089

Linting

Merge branch 'main' into pers-upstream

ad7deb9

[microNPU] Update Conv2D Tests to Use TF API to Gen Test Cases

6706248

Missing square brackets in parametrization, missing underscores.

Merge branch 'main' into pers-upstream

203cfd2

[microNPU] Update Conv2D Tests to Use TF API to Gen Test Cases

40da2da

CI- re trigger

dchauhan-arm force-pushed the pers-upstream branch from b86add3 to 40da2da Compare December 8, 2021 15:17

[microNPU] Update Conv2D Tests to Use TF API to Gen Test Cases

a950e35

Fix imports

ekalda approved these changes Dec 9, 2021

View reviewed changes

lhutton1 approved these changes Dec 9, 2021

View reviewed changes

manupak approved these changes Dec 9, 2021

View reviewed changes

manupak merged commit 1566fb1 into apache:main Dec 9, 2021

lhutton1 mentioned this pull request Dec 22, 2021

[microNPU] Remove remaining UnsupportedLayout checks #9791

Merged

manupak reviewed Jan 17, 2022

View reviewed changes

driazati mentioned this pull request Jul 14, 2022

TVM v0.9.0.rc0 Release Candidate Notes #12102

Closed

		"""This module defines all error types associated with the Arm(R) Ethos(TM)-U NPU code generator."""


		class EthosUCodegenError(Exception):

Conversation

dchauhan-arm commented Nov 15, 2021

Uh oh!

dchauhan-arm commented Nov 15, 2021

Uh oh!

manupak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ekalda left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

manupak commented Nov 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lhutton1 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ekalda left a comment

Choose a reason for hiding this comment

Uh oh!

ekalda Nov 25, 2021

Choose a reason for hiding this comment

Uh oh!

manupak Nov 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ekalda left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ekalda left a comment

Choose a reason for hiding this comment

Uh oh!

lhutton1 left a comment

Choose a reason for hiding this comment

Uh oh!

manupak left a comment

Choose a reason for hiding this comment

Uh oh!

manupak commented Dec 9, 2021

Uh oh!

manupak left a comment

Choose a reason for hiding this comment

Uh oh!

manupak Jan 17, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

manupak commented Nov 23, 2021 •

edited

Loading

lhutton1 left a comment •

edited

Loading

manupak Nov 25, 2021 •

edited

Loading