[BYOC][ETHOSN] Introduce further operator support #6355

mbaret · 2020-08-28T11:13:43Z

This PR introduces support for the following operators via the Ethos-N codegen:

Quantized Fully Connected
Quantized Addition
Depth-to-space
Max/Avg Pool 2D
Quantized Relu (Clip)
Reshape
Quantized Sigmoid

Additionally, tests for mobilenet, inceptionv3/4 and ssd mobilenet are added.

Co-authored-by: Leo Blonk Leo.Blonk@arm.com
Co-authored-by: Tristan O'Connor tristan.oconnor@arm.com
Co-authored-by: Ramana Radhakrishnan ramana.radhakrishnan@arm.com

mbaret · 2020-08-28T14:15:00Z

cc @Leo-arm @masahi @zhiics @comaniac

mbaret · 2020-09-01T08:35:28Z

@tqchen could you advise on what to do about the failing build? It seems one of the docker images doesn't have tensorflow available, so is there a way we can avoid running the tests there?

python/tvm/relay/op/contrib/ethosn.py

comaniac · 2020-09-04T23:19:26Z

src/runtime/contrib/ethosn/ethosn_runtime.cc

@@ -120,6 +120,14 @@ Module EthosnModule::LoadFromBinary(void* strm) {
  return Module(n);
 }

+void EthosnModule::SaveToFile(const std::string& path, const std::string& format) {


It looks me that this still saves a binary file. Are there specific use cases?

It's used by assert_lib_hash in the test infrastructure. See my comment further down on the purpose of the hashes.

tests/python/contrib/test_ethosn/test_addition.py

comaniac · 2020-09-05T00:15:14Z

tests/python/contrib/test_ethosn/test_networks.py

+                  "models/mobilenet_v1_2018_08_02/mobilenet_v1_1.0_224_quant.tgz",
+        model_sub_path="mobilenet_v1_1.0_224_quant.tflite",
+        input_dict={"input": (1, 224, 224, 3)},
+        compile_hash="81637c89339201a07dc96e3b5dbf836a",


How this hash comes from? In case someone in the future changes Ethos-N codegen, how can this be updated?

This is created by hashing the library output we get when compiling the model. We create the hash at a 'known good' point in time (i.e. when we run the HW tests internally and see everything is correct). The thinking here is that this provides a quick way to test whether changes in TVM have resulted in a behavioural change to our codegen.

We will remain responsible for updating these hashes while no public hardware is available. Should it become available, we'll no longer need the hashes as we can just test execution correctness directly.

I see. Could you comment this on the code with TODO? Thanks.

I've added in a comment rather than a TODO because I don't think this represents work that is yet to be done, but rather maintenance procedure. It explains the purpose of the hash and who to contact (myself and @Leo-arm ) should the test fail.

tests/python/contrib/test_ethosn/test_fullyconnected.py

leo-blonk · 2020-09-07T13:29:17Z

LGTM

masahi · 2020-09-09T01:27:29Z

@mbaret Can you act on @comaniac's comment?

zhiics · 2020-09-09T16:50:30Z

src/relay/backend/contrib/ethosn/codegen.cc

+  return EthosnError();
+}
+
+EthosnError ConstructNetworkVisitor::MakeReluLayer(const Call& call,


Just a minor suggestion that you feel free to ignore, I think these functions can be replaced by a macro as they look exactly the same and only operator name matters.

I think we won't reimplement this for now, but there are subtle differences between these functions, mostly around the number and type of the inputs/outputs + how quantization params should be handled.

tests/python/contrib/test_ethosn/test_networks.py

comaniac

Approved with concerns. Ideally, we hope everyone could jump in to maintain and improve a module if they have an interest. The current unit tests, however, can only be maintained by specific folks. I'll let others comment.

mbaret · 2020-09-09T21:35:32Z

This is an unfortunate consequence of the lack of hardware availability in CI. While that isn't available, it will necessarily be the case that we (Arm) need to maintain this code. I don't believe this is uncommon with new hardware in open source projects.

zhiics · 2020-09-11T23:21:20Z

@mbaret can you rebase?

mbaret · 2020-09-14T13:51:53Z

'Black' has definitely decreased the readability of some of our tests quite considerably. Might have been worth having tests be exempt...

zhiics

revert the vta changes

This PR introduces support for the following operators: - Quantized Fully Connected - Quantized Addition - Depth-to-space - Max/Avg Pool 2D - Quantized Relu (Clip) - Reshape - Quantized Sigmoid Co-authored-by: Leo Blonk <Leo.Blonk@arm.com> Co-authored-by: Tristan O'Connor <tristan.oconnor@arm.com> Co-authored-by: Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>

Change-Id: I11bcf4a78014fa63e7b8e3b0cb00eecfd6cb7760

Change-Id: I1fb1a11d0765f6d69f04c24b9c24e08665b8af6a

Change-Id: Id06063a0a0cf5f01356df23dc5d4bbbcb47cfa99

Change-Id: I330408dfabc4bd804373f100581ce909ff724052

Change-Id: I2c5007be485b323116a0e8bab0f9106ea5ec834b

Change-Id: I13828c918c959daa492b9ed942a882c86d6690d1

Change-Id: Idaa70ab9c2ec8db2828d51d15e7c23f28670ec82

Change-Id: I538171bd547a16395bef155a1dad28e8b3e347f2

mbaret · 2020-09-15T14:21:29Z

vta change reverted

mbaret · 2020-09-15T22:02:54Z

@zhiics @masahi is there anything left to do here?

zhiics · 2020-09-15T22:05:49Z

Thanks everyone!

mbaret · 2020-09-15T22:09:16Z

Thanks for all the reviews everyone :)

* [BYOC][ETHOSN] Introduce further operator support This PR introduces support for the following operators: - Quantized Fully Connected - Quantized Addition - Depth-to-space - Max/Avg Pool 2D - Quantized Relu (Clip) - Reshape - Quantized Sigmoid Co-authored-by: Leo Blonk <Leo.Blonk@arm.com> Co-authored-by: Tristan O'Connor <tristan.oconnor@arm.com> Co-authored-by: Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> * Skip tf imports if not available Change-Id: I11bcf4a78014fa63e7b8e3b0cb00eecfd6cb7760 * ethos -> ethosn Change-Id: I1fb1a11d0765f6d69f04c24b9c24e08665b8af6a * Reduce random testing in test_addition Change-Id: Id06063a0a0cf5f01356df23dc5d4bbbcb47cfa99 * Reduce random testing in test fullyconnected Change-Id: I330408dfabc4bd804373f100581ce909ff724052 * Fix dumb mistake with rename Change-Id: I2c5007be485b323116a0e8bab0f9106ea5ec834b * Added comments to update the hashes in network tests when necessary Change-Id: I13828c918c959daa492b9ed942a882c86d6690d1 * Fix github name Change-Id: Idaa70ab9c2ec8db2828d51d15e7c23f28670ec82 * Use black formatting Change-Id: I538171bd547a16395bef155a1dad28e8b3e347f2 Co-authored-by: Leo Blonk <Leo.Blonk@arm.com> Co-authored-by: Tristan O'Connor <tristan.oconnor@arm.com> Co-authored-by: Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>

tqchen added the status: need review label Sep 4, 2020

comaniac requested changes Sep 5, 2020

View reviewed changes

masahi approved these changes Sep 9, 2020

View reviewed changes

leo-blonk approved these changes Sep 9, 2020

View reviewed changes

zhiics reviewed Sep 9, 2020

View reviewed changes

comaniac approved these changes Sep 9, 2020

View reviewed changes

mbaret force-pushed the ethosn-ops branch from 058e435 to 6739000 Compare September 14, 2020 13:50

zhiics reviewed Sep 14, 2020

View reviewed changes

mbaret force-pushed the ethosn-ops branch from 6739000 to 2826cbf Compare September 15, 2020 12:00

mbaret and others added 9 commits September 15, 2020 14:21

Skip tf imports if not available

30a6b59

Change-Id: I11bcf4a78014fa63e7b8e3b0cb00eecfd6cb7760

ethos -> ethosn

2d1b5a9

Change-Id: I1fb1a11d0765f6d69f04c24b9c24e08665b8af6a

Reduce random testing in test_addition

e07ac24

Change-Id: Id06063a0a0cf5f01356df23dc5d4bbbcb47cfa99

Reduce random testing in test fullyconnected

894b781

Change-Id: I330408dfabc4bd804373f100581ce909ff724052

Fix dumb mistake with rename

b912c86

Change-Id: I2c5007be485b323116a0e8bab0f9106ea5ec834b

Added comments to update the hashes in network tests when necessary

a13f07f

Change-Id: I13828c918c959daa492b9ed942a882c86d6690d1

Fix github name

4b9c971

Change-Id: Idaa70ab9c2ec8db2828d51d15e7c23f28670ec82

Use black formatting

691322a

Change-Id: I538171bd547a16395bef155a1dad28e8b3e347f2

mbaret force-pushed the ethosn-ops branch from 2826cbf to 691322a Compare September 15, 2020 13:21

zhiics approved these changes Sep 15, 2020

View reviewed changes

zhiics merged commit 4c5f0c0 into apache:master Sep 15, 2020

zhiics added status: accepted and removed status: need review labels Sep 15, 2020

comaniac mentioned this pull request Sep 25, 2020

[RFC] v0.7 Release Planning #6421

Closed

ZihengJiang mentioned this pull request Sep 25, 2020

TVM v0.7 Release Note Candidate #6486

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BYOC][ETHOSN] Introduce further operator support #6355

[BYOC][ETHOSN] Introduce further operator support #6355

mbaret commented Aug 28, 2020

mbaret commented Aug 28, 2020

mbaret commented Sep 1, 2020

comaniac Sep 4, 2020

mbaret Sep 7, 2020

comaniac Sep 5, 2020

mbaret Sep 7, 2020

comaniac Sep 8, 2020

mbaret Sep 9, 2020

leo-blonk commented Sep 7, 2020

masahi commented Sep 9, 2020

zhiics Sep 9, 2020

mbaret Sep 9, 2020

comaniac left a comment

mbaret commented Sep 9, 2020

zhiics commented Sep 11, 2020

mbaret commented Sep 14, 2020

zhiics left a comment

mbaret commented Sep 15, 2020

mbaret commented Sep 15, 2020

zhiics commented Sep 15, 2020

mbaret commented Sep 15, 2020

[BYOC][ETHOSN] Introduce further operator support #6355

[BYOC][ETHOSN] Introduce further operator support #6355

Conversation

mbaret commented Aug 28, 2020

mbaret commented Aug 28, 2020

mbaret commented Sep 1, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leo-blonk commented Sep 7, 2020

masahi commented Sep 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

comaniac left a comment

Choose a reason for hiding this comment

mbaret commented Sep 9, 2020

zhiics commented Sep 11, 2020

mbaret commented Sep 14, 2020

zhiics left a comment

Choose a reason for hiding this comment

mbaret commented Sep 15, 2020

mbaret commented Sep 15, 2020

zhiics commented Sep 15, 2020

mbaret commented Sep 15, 2020