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 2021/11/16 11:03:39 UTC
[GitHub] [tvm] lhutton1 opened a new pull request #9515: [microNPU] Allow constants to be given as input to an operator
lhutton1 opened a new pull request #9515:
URL: https://github.com/apache/tvm/pull/9515
Currently the expectation is that all constants need to be encoded, however, this is not always the case for scalar inputs. This PR ensures that constants that don't need encoding are not treated like encoded constants by the EncodeConstants pass.
cc @mbaret @manupa-arm @ekalda @NicolaLancellotti @dchauhan-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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] lhutton1 commented on a change in pull request #9515: [microNPU] Allow constants to be given as input to an operator
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #9515:
URL: https://github.com/apache/tvm/pull/9515#discussion_r750507953
##########
File path: tests/python/contrib/test_ethosu/test_encode_constants.py
##########
@@ -270,5 +273,47 @@ def _get_func():
assert reference_const_sizes == test_const_sizes
+def test_constant_as_input():
+ """Test to check that constants specified as inputs aren't
+ interpreted as an encoded constant."""
+
+ def get_graph():
+ dtype = "uint8"
Review comment:
For now this needs to be `uint8` due to https://github.com/apache/tvm/blob/main/python/tvm/relay/backend/contrib/ethosu/tir_to_cs_translator.py#L254. Removing this opens up another can of worms that I'll address in another PR :)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] manupa-arm merged pull request #9515: [microNPU] Allow constants to be given as input to an operator
Posted by GitBox <gi...@apache.org>.
manupa-arm merged pull request #9515:
URL: https://github.com/apache/tvm/pull/9515
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] ekalda commented on a change in pull request #9515: [microNPU] Allow constants to be given as input to an operator
Posted by GitBox <gi...@apache.org>.
ekalda commented on a change in pull request #9515:
URL: https://github.com/apache/tvm/pull/9515#discussion_r751049520
##########
File path: tests/python/contrib/test_ethosu/test_encode_constants.py
##########
@@ -270,5 +273,47 @@ def _get_func():
assert reference_const_sizes == test_const_sizes
+def test_constant_as_input():
+ """Test to check that constants specified as inputs aren't
+ interpreted as an encoded constant."""
+
+ def get_graph():
+ dtype = "uint8"
Review comment:
Yes, makes sense, thanks for clarifying! :)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] lhutton1 commented on a change in pull request #9515: [microNPU] Allow constants to be given as input to an operator
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #9515:
URL: https://github.com/apache/tvm/pull/9515#discussion_r750511584
##########
File path: tests/python/contrib/test_ethosu/test_codegen.py
##########
@@ -435,6 +435,56 @@ def representative_dataset():
infra.verify_source(compiled_models, accel_type)
+@pytest.mark.parametrize("accel_type", ACCEL_TYPES)
+def test_binary_add_from_constant_scalar(accel_type):
+ dtype = "uint8"
+ ifm_shape = (1, 4, 4, 8)
+
+ def create_relay_graph():
+ inp = relay.var("input", shape=ifm_shape, dtype=dtype)
+ scalar = relay.const(np.ones((1, 1, 1, 1), dtype=dtype), dtype=dtype)
+ add = relay.qnn.op.add(
+ inp,
+ scalar,
+ relay.const(1.0, dtype="float32"),
+ relay.const(0, dtype="int32"),
+ relay.const(1.0, dtype="float32"),
+ relay.const(0, dtype="int32"),
+ relay.const(1.0, dtype="float32"),
+ relay.const(0, dtype="int32"),
+ )
+ func = relay.Function(relay.analysis.free_vars(add), add)
Review comment:
I wasn't sure if there was a way to generate similar relay from TFLite, although admittedly I didn't really check. I'll have a look into it.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] NicolaLancellotti commented on a change in pull request #9515: [microNPU] Allow constants to be given as input to an operator
Posted by GitBox <gi...@apache.org>.
NicolaLancellotti commented on a change in pull request #9515:
URL: https://github.com/apache/tvm/pull/9515#discussion_r750247811
##########
File path: tests/python/contrib/test_ethosu/test_legalize.py
##########
@@ -693,6 +693,53 @@ def verify(ext_func):
verify(mod["tvmgen_default_ethos_u_main_0"])
+def test_binary_add_from_constant_scalar():
+ dtype = "uint8"
+ ifm_shape = (1, 4, 4, 8)
+
+ def create_graph():
+ inp = relay.var("input", shape=ifm_shape, dtype=dtype)
+ scalar = relay.const(np.ones((1, 1, 1, 1), dtype=dtype), dtype=dtype)
+ add = relay.qnn.op.add(
+ inp,
+ scalar,
+ relay.const(1.0, dtype="float32"),
+ relay.const(0, dtype="int32"),
+ relay.const(1.0, dtype="float32"),
+ relay.const(0, dtype="int32"),
+ relay.const(1.0, dtype="float32"),
+ relay.const(0, dtype="int32"),
+ )
+ func = relay.Function(relay.analysis.free_vars(add), add)
+ return tvm.IRModule.from_expr(func)
+
+ def verify(ext_func):
+ op = ext_func.body
+ assert list(op.args[0].checked_type.shape) == [1, 4, 4, 8]
+ assert list(op.args[1].checked_type.shape) == [1, 1, 1, 1]
+ assert op.args[0].checked_type.dtype == "uint8"
+ assert list(op.checked_type.shape) == [1, 4, 4, 8]
+ assert op.checked_type.dtype == "uint8"
+ assert op.attrs.operator_type == "ADD"
+
+ rewriter = legalize.AddRewriter()
+ pattern_table = [
+ (
+ ethosu.AddParams.composite_name,
+ ethosu.qnn_add_pattern(),
+ lambda pat: ethosu.AddParams(pat).is_valid(),
+ ),
+ ]
+
+ mod = create_graph()
+ mod = partition_ethosu_by_table(mod, pattern_table)
+
+ mod["tvmgen_default_ethosu_main_0"] = dataflow_pattern.rewrite(
+ rewriter, mod["tvmgen_default_ethosu_main_0"]
+ )
+ verify(mod["tvmgen_default_ethosu_main_0"])
Review comment:
```suggestion
mod["tvmgen_default_ethos_u_main_0"] = dataflow_pattern.rewrite(
rewriter, mod["tvmgen_default_ethos_u_main_0"]
)
verify(mod["tvmgen_default_ethos_u_main_0"])
```
##########
File path: tests/python/contrib/test_ethosu/test_encode_constants.py
##########
@@ -270,5 +273,47 @@ def _get_func():
assert reference_const_sizes == test_const_sizes
+def test_constant_as_input():
+ """Test to check that constants specified as inputs aren't
+ interpreted as an encoded constant."""
+
+ def get_graph():
+ dtype = "uint8"
+ ifm = relay.var("ifm", shape=(1, 16, 16, 32), dtype=dtype)
+ conv1 = make_ethosu_conv2d(
+ ifm,
+ 32,
+ 16,
+ (1, 1),
+ (0, 0),
+ (1, 1),
+ (1, 1),
+ )
+ scalar = relay.const(np.ones((1, 1, 1, 1), dtype=dtype), dtype=dtype)
+ add1 = make_ethosu_binary_elementwise(
+ conv1, scalar, ifm_channels=32, ifm2_channels=1, operator_type="ADD", ofm_dtype=dtype
+ )
+ func = relay.Function(relay.analysis.free_vars(add1), add1)
+ func = run_opt_pass(func, relay.transform.InferType())
+ return func
+
+ tir_mod, params = lower_to_tir(get_graph(), copy_constants())
+
+ # Check tile address for the scalar constant input hasn't been
+ # overwritten.
+ extern_calls = tir_mod["main"].body.body.body.body.body
+ binary_elmtwise = extern_calls[-1].value
+ args = binary_elmtwise.args
Review comment:
```suggestion
binary_elementwise = extern_calls[-1].value
args = binary_elementwise.args
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] lhutton1 commented on a change in pull request #9515: [microNPU] Allow constants to be given as input to an operator
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #9515:
URL: https://github.com/apache/tvm/pull/9515#discussion_r750534459
##########
File path: tests/python/contrib/test_ethosu/test_codegen.py
##########
@@ -435,6 +435,56 @@ def representative_dataset():
infra.verify_source(compiled_models, accel_type)
+@pytest.mark.parametrize("accel_type", ACCEL_TYPES)
+def test_binary_add_from_constant_scalar(accel_type):
+ dtype = "uint8"
+ ifm_shape = (1, 4, 4, 8)
+
+ def create_relay_graph():
+ inp = relay.var("input", shape=ifm_shape, dtype=dtype)
+ scalar = relay.const(np.ones((1, 1, 1, 1), dtype=dtype), dtype=dtype)
+ add = relay.qnn.op.add(
+ inp,
+ scalar,
+ relay.const(1.0, dtype="float32"),
+ relay.const(0, dtype="int32"),
+ relay.const(1.0, dtype="float32"),
+ relay.const(0, dtype="int32"),
+ relay.const(1.0, dtype="float32"),
+ relay.const(0, dtype="int32"),
+ )
+ func = relay.Function(relay.analysis.free_vars(add), add)
Review comment:
Ah, Relay is also needed due to the `uint8` restriction so I'll leave this for 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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] ekalda commented on a change in pull request #9515: [microNPU] Allow constants to be given as input to an operator
Posted by GitBox <gi...@apache.org>.
ekalda commented on a change in pull request #9515:
URL: https://github.com/apache/tvm/pull/9515#discussion_r751049064
##########
File path: tests/python/contrib/test_ethosu/test_codegen.py
##########
@@ -435,6 +435,56 @@ def representative_dataset():
infra.verify_source(compiled_models, accel_type)
+@pytest.mark.parametrize("accel_type", ACCEL_TYPES)
+def test_binary_add_from_constant_scalar(accel_type):
+ dtype = "uint8"
+ ifm_shape = (1, 4, 4, 8)
+
+ def create_relay_graph():
+ inp = relay.var("input", shape=ifm_shape, dtype=dtype)
+ scalar = relay.const(np.ones((1, 1, 1, 1), dtype=dtype), dtype=dtype)
+ add = relay.qnn.op.add(
+ inp,
+ scalar,
+ relay.const(1.0, dtype="float32"),
+ relay.const(0, dtype="int32"),
+ relay.const(1.0, dtype="float32"),
+ relay.const(0, dtype="int32"),
+ relay.const(1.0, dtype="float32"),
+ relay.const(0, dtype="int32"),
+ )
+ func = relay.Function(relay.analysis.free_vars(add), add)
Review comment:
Ok yes, that makes sense!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] ekalda commented on a change in pull request #9515: [microNPU] Allow constants to be given as input to an operator
Posted by GitBox <gi...@apache.org>.
ekalda commented on a change in pull request #9515:
URL: https://github.com/apache/tvm/pull/9515#discussion_r750487693
##########
File path: tests/python/contrib/test_ethosu/test_codegen.py
##########
@@ -435,6 +435,56 @@ def representative_dataset():
infra.verify_source(compiled_models, accel_type)
+@pytest.mark.parametrize("accel_type", ACCEL_TYPES)
+def test_binary_add_from_constant_scalar(accel_type):
+ dtype = "uint8"
+ ifm_shape = (1, 4, 4, 8)
+
+ def create_relay_graph():
+ inp = relay.var("input", shape=ifm_shape, dtype=dtype)
+ scalar = relay.const(np.ones((1, 1, 1, 1), dtype=dtype), dtype=dtype)
+ add = relay.qnn.op.add(
+ inp,
+ scalar,
+ relay.const(1.0, dtype="float32"),
+ relay.const(0, dtype="int32"),
+ relay.const(1.0, dtype="float32"),
+ relay.const(0, dtype="int32"),
+ relay.const(1.0, dtype="float32"),
+ relay.const(0, dtype="int32"),
+ )
+ func = relay.Function(relay.analysis.free_vars(add), add)
Review comment:
Is there a reason we start from Relay there instead of TFLite?
##########
File path: tests/python/contrib/test_ethosu/test_encode_constants.py
##########
@@ -270,5 +273,47 @@ def _get_func():
assert reference_const_sizes == test_const_sizes
+def test_constant_as_input():
+ """Test to check that constants specified as inputs aren't
+ interpreted as an encoded constant."""
+
+ def get_graph():
+ dtype = "uint8"
Review comment:
Why does the constant need to be`uint8`? (just asking for enlightenment)
##########
File path: tests/python/contrib/test_ethosu/test_codegen.py
##########
@@ -435,6 +435,56 @@ def representative_dataset():
infra.verify_source(compiled_models, accel_type)
+@pytest.mark.parametrize("accel_type", ACCEL_TYPES)
+def test_binary_add_from_constant_scalar(accel_type):
+ dtype = "uint8"
+ ifm_shape = (1, 4, 4, 8)
+
+ def create_relay_graph():
+ inp = relay.var("input", shape=ifm_shape, dtype=dtype)
+ scalar = relay.const(np.ones((1, 1, 1, 1), dtype=dtype), dtype=dtype)
+ add = relay.qnn.op.add(
+ inp,
+ scalar,
+ relay.const(1.0, dtype="float32"),
+ relay.const(0, dtype="int32"),
+ relay.const(1.0, dtype="float32"),
+ relay.const(0, dtype="int32"),
+ relay.const(1.0, dtype="float32"),
+ relay.const(0, dtype="int32"),
+ )
+ func = relay.Function(relay.analysis.free_vars(add), add)
+ return tvm.IRModule.from_expr(func)
+
+ mod = create_relay_graph()
+ partitioned_mod = partition_for_ethosu(mod)
+
+ # Generate reference data
+ input_data = {"input": np.random.randint(low=0, high=255, size=ifm_shape, dtype=dtype)}
+ output_data = generate_ref_data(mod, input_data)
+
+ compiled_models = infra.build_source(
+ partitioned_mod,
+ input_data,
+ output_data,
+ accel_type,
+ output_tolerance=0,
+ )
+
+ # Assumes only two runtime.Modules are created -- i.e. single offload module
+ imported_modules = compiled_models[0].executor_factory.lib.imported_modules
+ assert len(imported_modules) == 2
+ ethosu_module = imported_modules[0]
+
+ # Verify generated C source
+ get_cs = tvm._ffi.get_global_func("runtime.module.ethosu.getcs")
Review comment:
```suggestion
get_cs = tvm._ffi.get_global_func("runtime.module.ethos-u.getcs")
```
Looks like 'ethos-u' is all the rage 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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] manupa-arm commented on pull request #9515: [microNPU] Allow constants to be given as input to an operator
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on pull request #9515:
URL: https://github.com/apache/tvm/pull/9515#issuecomment-971397872
Thanks @lhutton1 @ekalda
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org