You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/09/25 11:20:33 UTC
[GitHub] [incubator-tvm] mbaret opened a new pull request #6560: [BYOC][ETHOSN] Fix tests for new module API
mbaret opened a new pull request #6560:
URL: https://github.com/apache/incubator-tvm/pull/6560
Some of the downstream variants of our tests had been broken by a recent change to the API of build. This both fixes that and refactors a couple of tests so that they will run entirely in upstream CI and we won't see this sort of failure again.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mbaret commented on a change in pull request #6560: [BYOC][ETHOSN] Fix tests for new module API
Posted by GitBox <gi...@apache.org>.
mbaret commented on a change in pull request #6560:
URL: https://github.com/apache/incubator-tvm/pull/6560#discussion_r496106297
##########
File path: tests/python/contrib/test_ethosn/test_networks.py
##########
@@ -104,17 +96,16 @@ def test_mobilenet_v1():
# codegen, which could come about from either a change in Support Library
# version or a change in the Ethos-N codegen. To update this requires running
# on hardware that isn't available in CI.
- hw = ethosn_available()
_test_image_network(
model_url="https://storage.googleapis.com/download.tensorflow.org/"
"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",
output_count=1,
- run=(hw == Available.SW_AND_HW),
host_ops=3,
npu_partitions=1,
+ run=True,
Review comment:
We do run that particular one because it's small :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] manupa-arm commented on a change in pull request #6560: [BYOC][ETHOSN] Fix tests for new module API
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on a change in pull request #6560:
URL: https://github.com/apache/incubator-tvm/pull/6560#discussion_r497417303
##########
File path: tests/python/contrib/test_ethosn/test_networks.py
##########
@@ -104,17 +96,16 @@ def test_mobilenet_v1():
# codegen, which could come about from either a change in Support Library
# version or a change in the Ethos-N codegen. To update this requires running
# on hardware that isn't available in CI.
- hw = ethosn_available()
_test_image_network(
model_url="https://storage.googleapis.com/download.tensorflow.org/"
"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",
output_count=1,
- run=(hw == Available.SW_AND_HW),
host_ops=3,
npu_partitions=1,
+ run=True,
Review comment:
Thanks! all good now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mbaret commented on a change in pull request #6560: [BYOC][ETHOSN] Fix tests for new module API
Posted by GitBox <gi...@apache.org>.
mbaret commented on a change in pull request #6560:
URL: https://github.com/apache/incubator-tvm/pull/6560#discussion_r496106047
##########
File path: tests/python/contrib/test_ethosn/test_networks.py
##########
@@ -78,24 +78,16 @@ def get_model():
)
return _get_tflite_model(model_path, input_dict, "uint8")
- outputs = []
inputs = {}
for input_name in input_dict:
input_shape = input_dict[input_name]
inputs[input_name] = tei.get_real_image(input_shape[1], input_shape[2])
- for npu in [False, True]:
- mod, params = get_model()
- graph, lib, params = tei.build(
- mod, params, npu=npu, expected_host_ops=host_ops, npu_partitions=npu_partitions
- )
- if npu:
- tei.assert_lib_hash(lib, compile_hash)
- if run:
- outputs.append(tei.run(graph, lib, params, inputs, output_count, npu=npu))
-
+ mod, params = get_model()
+ m = tei.build(mod, params, npu=True, expected_host_ops=host_ops, npu_partitions=npu_partitions)
Review comment:
I've improved the docs a bit to better explain what npu does.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] manupa-arm commented on pull request #6560: [BYOC][ETHOSN] Fix tests for new module API
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on pull request #6560:
URL: https://github.com/apache/incubator-tvm/pull/6560#issuecomment-701314286
LGTM
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mbaret commented on a change in pull request #6560: [BYOC][ETHOSN] Fix tests for new module API
Posted by GitBox <gi...@apache.org>.
mbaret commented on a change in pull request #6560:
URL: https://github.com/apache/incubator-tvm/pull/6560#discussion_r497388809
##########
File path: tests/python/contrib/test_ethosn/test_networks.py
##########
@@ -104,17 +96,16 @@ def test_mobilenet_v1():
# codegen, which could come about from either a change in Support Library
# version or a change in the Ethos-N codegen. To update this requires running
# on hardware that isn't available in CI.
- hw = ethosn_available()
_test_image_network(
model_url="https://storage.googleapis.com/download.tensorflow.org/"
"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",
output_count=1,
- run=(hw == Available.SW_AND_HW),
host_ops=3,
npu_partitions=1,
+ run=True,
Review comment:
Updated docs to explain what run does in this context.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mbaret commented on a change in pull request #6560: [BYOC][ETHOSN] Fix tests for new module API
Posted by GitBox <gi...@apache.org>.
mbaret commented on a change in pull request #6560:
URL: https://github.com/apache/incubator-tvm/pull/6560#discussion_r495817481
##########
File path: tests/python/contrib/test_ethosn/test_networks.py
##########
@@ -78,24 +78,16 @@ def get_model():
)
return _get_tflite_model(model_path, input_dict, "uint8")
- outputs = []
inputs = {}
for input_name in input_dict:
input_shape = input_dict[input_name]
inputs[input_name] = tei.get_real_image(input_shape[1], input_shape[2])
- for npu in [False, True]:
- mod, params = get_model()
- graph, lib, params = tei.build(
- mod, params, npu=npu, expected_host_ops=host_ops, npu_partitions=npu_partitions
- )
- if npu:
- tei.assert_lib_hash(lib, compile_hash)
- if run:
- outputs.append(tei.run(graph, lib, params, inputs, output_count, npu=npu))
-
+ mod, params = get_model()
+ m = tei.build(mod, params, npu=True, expected_host_ops=host_ops, npu_partitions=npu_partitions)
Review comment:
It will never be false for these tests, but it is for others like test_conv2d.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] manupa-arm commented on a change in pull request #6560: [BYOC][ETHOSN] Fix tests for new module API
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on a change in pull request #6560:
URL: https://github.com/apache/incubator-tvm/pull/6560#discussion_r496564570
##########
File path: tests/python/contrib/test_ethosn/test_networks.py
##########
@@ -104,17 +96,16 @@ def test_mobilenet_v1():
# codegen, which could come about from either a change in Support Library
# version or a change in the Ethos-N codegen. To update this requires running
# on hardware that isn't available in CI.
- hw = ethosn_available()
_test_image_network(
model_url="https://storage.googleapis.com/download.tensorflow.org/"
"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",
output_count=1,
- run=(hw == Available.SW_AND_HW),
host_ops=3,
npu_partitions=1,
+ run=True,
Review comment:
I think its worth mentioning the functionality of inference_result as registered as ""relay.ethos-n.test.infra.inference_result" has two faces :) -- based on the availability of the hardware, in the tei.
Otherwise, its hard to understand what's this "run" really means and the need for the assert_lib_hash.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] mbaret commented on pull request #6560: [BYOC][ETHOSN] Fix tests for new module API
Posted by GitBox <gi...@apache.org>.
mbaret commented on pull request #6560:
URL: https://github.com/apache/incubator-tvm/pull/6560#issuecomment-699875770
cc @Leo-arm could you take a look?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] comaniac commented on a change in pull request #6560: [BYOC][ETHOSN] Fix tests for new module API
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #6560:
URL: https://github.com/apache/incubator-tvm/pull/6560#discussion_r495130579
##########
File path: tests/python/contrib/test_ethosn/test_networks.py
##########
@@ -78,24 +78,16 @@ def get_model():
)
return _get_tflite_model(model_path, input_dict, "uint8")
- outputs = []
inputs = {}
for input_name in input_dict:
input_shape = input_dict[input_name]
inputs[input_name] = tei.get_real_image(input_shape[1], input_shape[2])
- for npu in [False, True]:
- mod, params = get_model()
- graph, lib, params = tei.build(
- mod, params, npu=npu, expected_host_ops=host_ops, npu_partitions=npu_partitions
- )
- if npu:
- tei.assert_lib_hash(lib, compile_hash)
- if run:
- outputs.append(tei.run(graph, lib, params, inputs, output_count, npu=npu))
-
+ mod, params = get_model()
+ m = tei.build(mod, params, npu=True, expected_host_ops=host_ops, npu_partitions=npu_partitions)
Review comment:
After looking into `build`, it seems like it just doesn't go through the Ethos-N partition when `npu=False` but just equivalent to `relay.build`. Accordingly,
- Looks like `npu` will never be False. Do we need to keep this argument?
- If we need to keep this argument, maybe we need better naming such as `with_npu`. `npu` is somewhat confusing.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] comaniac merged pull request #6560: [BYOC][ETHOSN] Fix tests for new module API
Posted by GitBox <gi...@apache.org>.
comaniac merged pull request #6560:
URL: https://github.com/apache/incubator-tvm/pull/6560
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] comaniac commented on pull request #6560: [BYOC][ETHOSN] Fix tests for new module API
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #6560:
URL: https://github.com/apache/incubator-tvm/pull/6560#issuecomment-701535931
Thanks @mbaret @manupa-arm @Leo-arm
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] manupa-arm commented on a change in pull request #6560: [BYOC][ETHOSN] Fix tests for new module API
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on a change in pull request #6560:
URL: https://github.com/apache/incubator-tvm/pull/6560#discussion_r495822850
##########
File path: tests/python/contrib/test_ethosn/test_networks.py
##########
@@ -104,17 +96,16 @@ def test_mobilenet_v1():
# codegen, which could come about from either a change in Support Library
# version or a change in the Ethos-N codegen. To update this requires running
# on hardware that isn't available in CI.
- hw = ethosn_available()
_test_image_network(
model_url="https://storage.googleapis.com/download.tensorflow.org/"
"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",
output_count=1,
- run=(hw == Available.SW_AND_HW),
host_ops=3,
npu_partitions=1,
+ run=True,
Review comment:
I thought we are not running anything ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org