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/06/14 15:46:54 UTC
[GitHub] [tvm] leandron opened a new pull request #8253: [tvmc] Add a --config option to `tvmc compile`
leandron opened a new pull request #8253:
URL: https://github.com/apache/tvm/pull/8253
[tvmc] Add a `--config` option to `tvmc compile`:
* Allow to send some configurations to the `PassContext` via command line
* Add various validations to the new option with appropriate error messages
* Add unit testing
cc @gromero @comaniac @manupa-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] [tvm] leandron commented on a change in pull request #8253: [tvmc] Add a --config option to `tvmc compile`
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #8253:
URL: https://github.com/apache/tvm/pull/8253#discussion_r651587235
##########
File path: tests/python/driver/tvmc/test_tvmc_common.py
##########
@@ -306,3 +306,49 @@ def test_parse_quotes_and_separators_on_options():
assert len(targets_double_quote) == 1
assert "+v1.0x,+value" == targets_double_quote[0]["opts"]["option1"]
+
+
+def test_config_invalid_format():
+ with pytest.raises(TVMCException):
+ _ = tvmc.common.parse_configs(["relay.backend.use_auto_scheduler.missing.value"])
+
+
+def test_config_missing_from_tvm():
+ with pytest.raises(TVMCException):
+ _ = tvmc.common.parse_configs(["relay.backend.use_auto_scheduler.missing.value=1234"])
+
+
+def test_config_unsupported_tvmc_config():
+ with pytest.raises(TVMCException):
+ _ = tvmc.common.parse_configs(["tir.LoopPartition=value"])
+
+
+def test_config_empty():
+ with pytest.raises(TVMCException):
+ _ = tvmc.common.parse_configs([""])
+
+
+def test_config_valid_config_bool():
+ configs = tvmc.common.parse_configs(["relay.backend.use_auto_scheduler=true"])
+
+ assert len(configs) == 1
+ assert "relay.backend.use_auto_scheduler" in configs.keys()
+ assert configs["relay.backend.use_auto_scheduler"] == True
+
+
+def test_config_valid_multiple_configs():
+ configs = tvmc.common.parse_configs(
+ [
+ "relay.backend.use_auto_scheduler=false",
+ "tir.detect_global_barrier=10",
+ "relay.ext.vitis_ai.options.build_dir=mystring",
Review comment:
Yeah, it crashed because that specific CI environment doesn't build with Vitis support ON. I'll need to use a different config name for my test.
edit: Actually, it turns out that vitis configs are the only ones using string type at the moment, so I make this specific test case to be conditional.
--
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] [tvm] gromero commented on a change in pull request #8253: [tvmc] Add a --config option to `tvmc compile`
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #8253:
URL: https://github.com/apache/tvm/pull/8253#discussion_r651769989
##########
File path: python/tvm/driver/tvmc/compiler.py
##########
@@ -42,6 +42,13 @@ def add_compile_parser(subparsers):
parser = subparsers.add_parser("compile", help="compile a model.")
parser.set_defaults(func=drive_compile)
+ parser.add_argument(
+ "--config",
+ action="append",
+ metavar=("name=value"),
+ help="configurations to be used at compile time. A subset of options provided "
+ "by TVM are supported. e.g. 'relay.backend.use_auto_scheduler=0'",
Review comment:
OK. Yeah I was indeed thinking of how we deal with `--target` ;)
--
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] [tvm] leandron commented on a change in pull request #8253: [tvmc] Add a --config option to `tvmc compile`
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #8253:
URL: https://github.com/apache/tvm/pull/8253#discussion_r651588641
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -415,3 +415,86 @@ def parse_shape_string(inputs_string):
shape_dict[name] = shape
return shape_dict
+
+
+def set_config_value(name, value, config_type):
Review comment:
Makes sense, I updated that 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] [tvm] leandron commented on a change in pull request #8253: [tvmc] Add a --config option to `tvmc compile`
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #8253:
URL: https://github.com/apache/tvm/pull/8253#discussion_r651616441
##########
File path: python/tvm/driver/tvmc/compiler.py
##########
@@ -42,6 +42,13 @@ def add_compile_parser(subparsers):
parser = subparsers.add_parser("compile", help="compile a model.")
parser.set_defaults(func=drive_compile)
+ parser.add_argument(
+ "--config",
Review comment:
I'll move it to be `pass-config` instead.
--
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] [tvm] leandron commented on a change in pull request #8253: [tvmc] Add a --config option to `tvmc compile`
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #8253:
URL: https://github.com/apache/tvm/pull/8253#discussion_r651587235
##########
File path: tests/python/driver/tvmc/test_tvmc_common.py
##########
@@ -306,3 +306,49 @@ def test_parse_quotes_and_separators_on_options():
assert len(targets_double_quote) == 1
assert "+v1.0x,+value" == targets_double_quote[0]["opts"]["option1"]
+
+
+def test_config_invalid_format():
+ with pytest.raises(TVMCException):
+ _ = tvmc.common.parse_configs(["relay.backend.use_auto_scheduler.missing.value"])
+
+
+def test_config_missing_from_tvm():
+ with pytest.raises(TVMCException):
+ _ = tvmc.common.parse_configs(["relay.backend.use_auto_scheduler.missing.value=1234"])
+
+
+def test_config_unsupported_tvmc_config():
+ with pytest.raises(TVMCException):
+ _ = tvmc.common.parse_configs(["tir.LoopPartition=value"])
+
+
+def test_config_empty():
+ with pytest.raises(TVMCException):
+ _ = tvmc.common.parse_configs([""])
+
+
+def test_config_valid_config_bool():
+ configs = tvmc.common.parse_configs(["relay.backend.use_auto_scheduler=true"])
+
+ assert len(configs) == 1
+ assert "relay.backend.use_auto_scheduler" in configs.keys()
+ assert configs["relay.backend.use_auto_scheduler"] == True
+
+
+def test_config_valid_multiple_configs():
+ configs = tvmc.common.parse_configs(
+ [
+ "relay.backend.use_auto_scheduler=false",
+ "tir.detect_global_barrier=10",
+ "relay.ext.vitis_ai.options.build_dir=mystring",
Review comment:
Yeah, it crashed because that specific CI environment doesn't build with Vitis support ON. I'll need to use a different config name for my test.
--
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] [tvm] leandron commented on a change in pull request #8253: [tvmc] Add a --config option to `tvmc compile`
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #8253:
URL: https://github.com/apache/tvm/pull/8253#discussion_r651581634
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -415,3 +415,86 @@ def parse_shape_string(inputs_string):
shape_dict[name] = shape
return shape_dict
+
+
+def set_config_value(name, value, config_type):
+ """Set a PassContext configuration value according to its value"""
+
+ if config_type == "IntImm":
+ # "Bool" configurations in the PassContext are recognized as
+ # IntImm, so deal with this case here
+ mapping_values = {
+ "false": False,
+ "true": True,
+ }
+
+ if value.isdigit():
+ parsed_value = int(value)
+ else:
+ # if not an int, accept only values on the mapping table, case insensitive
+ parsed_value = mapping_values.get(value.lower(), None)
+
+ if parsed_value is None:
+ raise TVMCException(f"Invalid value '{value}' for configuration '{name}'. ")
+
+ if config_type == "runtime.String":
+ parsed_value = value
+
+ return parsed_value
+
+
+def parse_configs(input_configs):
+ """Parse configuration values set via command line.
+
+ Parameters
+ ----------
+ input_configs: list of str
+ list of configurations provided via command line.
+
+ Returns
+ -------
+ pass_context_configs: dict
+ a dict containing key-value configs to be used in the PassContext.
+ """
+ all_configs = tvm.ir.transform.PassContext.list_configs()
+ supported_config_types = ("IntImm", "runtime.String")
+ supported_configs = [
+ name for name in all_configs.keys() if all_configs[name]["type"] in supported_config_types
+ ]
+ pass_context_configs = {}
+
+ if not input_configs:
+ return {}
+
+ for config in input_configs:
+ if len(config) == 0:
Review comment:
Done
--
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] [tvm] leandron commented on a change in pull request #8253: [tvmc] Add a --config option to `tvmc compile`
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #8253:
URL: https://github.com/apache/tvm/pull/8253#discussion_r651615824
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -415,3 +415,86 @@ def parse_shape_string(inputs_string):
shape_dict[name] = shape
return shape_dict
+
+
+def set_config_value(name, value, config_type):
+ """Set a PassContext configuration value according to its value"""
+
+ if config_type == "IntImm":
+ # "Bool" configurations in the PassContext are recognized as
+ # IntImm, so deal with this case here
+ mapping_values = {
+ "false": False,
+ "true": True,
+ }
+
+ if value.isdigit():
+ parsed_value = int(value)
+ else:
+ # if not an int, accept only values on the mapping table, case insensitive
+ parsed_value = mapping_values.get(value.lower(), None)
+
+ if parsed_value is None:
+ raise TVMCException(f"Invalid value '{value}' for configuration '{name}'. ")
+
+ if config_type == "runtime.String":
+ parsed_value = value
Review comment:
I did some investigation on this, and wrt to the `distutils` I didn't want to add that dependency here, because I think it would be a a bit misplaced.
Also wrt to the `json` approach, I think it would still require more validation because the allowed values for that option are "int numbers", "true" or "false", and opening that to "json.loads" would add all sorts of json passing, then requiring more validation. That's why I added my own mapping table.
--
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] [tvm] comaniac commented on a change in pull request #8253: [tvmc] Add a --config option to `tvmc compile`
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #8253:
URL: https://github.com/apache/tvm/pull/8253#discussion_r652165844
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -415,3 +415,86 @@ def parse_shape_string(inputs_string):
shape_dict[name] = shape
return shape_dict
+
+
+def set_config_value(name, value, config_type):
+ """Set a PassContext configuration value according to its value"""
+
+ if config_type == "IntImm":
+ # "Bool" configurations in the PassContext are recognized as
+ # IntImm, so deal with this case here
+ mapping_values = {
+ "false": False,
+ "true": True,
+ }
+
+ if value.isdigit():
+ parsed_value = int(value)
+ else:
+ # if not an int, accept only values on the mapping table, case insensitive
+ parsed_value = mapping_values.get(value.lower(), None)
+
+ if parsed_value is None:
+ raise TVMCException(f"Invalid value '{value}' for configuration '{name}'. ")
+
+ if config_type == "runtime.String":
+ parsed_value = value
Review comment:
Hmm you already processed int so I don't think it would be an issue here. Anyways, I don't have a strong preference of using `json` so I'll commit.
--
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] [tvm] comaniac commented on a change in pull request #8253: [tvmc] Add a --config option to `tvmc compile`
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #8253:
URL: https://github.com/apache/tvm/pull/8253#discussion_r651120543
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -415,3 +415,86 @@ def parse_shape_string(inputs_string):
shape_dict[name] = shape
return shape_dict
+
+
+def set_config_value(name, value, config_type):
+ """Set a PassContext configuration value according to its value"""
+
+ if config_type == "IntImm":
+ # "Bool" configurations in the PassContext are recognized as
+ # IntImm, so deal with this case here
+ mapping_values = {
+ "false": False,
+ "true": True,
+ }
+
+ if value.isdigit():
+ parsed_value = int(value)
+ else:
+ # if not an int, accept only values on the mapping table, case insensitive
+ parsed_value = mapping_values.get(value.lower(), None)
+
+ if parsed_value is None:
+ raise TVMCException(f"Invalid value '{value}' for configuration '{name}'. ")
+
+ if config_type == "runtime.String":
+ parsed_value = value
+
+ return parsed_value
+
+
+def parse_configs(input_configs):
+ """Parse configuration values set via command line.
+
+ Parameters
+ ----------
+ input_configs: list of str
+ list of configurations provided via command line.
+
+ Returns
+ -------
+ pass_context_configs: dict
+ a dict containing key-value configs to be used in the PassContext.
+ """
+ all_configs = tvm.ir.transform.PassContext.list_configs()
+ supported_config_types = ("IntImm", "runtime.String")
+ supported_configs = [
+ name for name in all_configs.keys() if all_configs[name]["type"] in supported_config_types
+ ]
+ pass_context_configs = {}
+
+ if not input_configs:
+ return {}
Review comment:
Move this to the beginning of this function so that you don't need to process all available configs if users don't specify any.
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -415,3 +415,86 @@ def parse_shape_string(inputs_string):
shape_dict[name] = shape
return shape_dict
+
+
+def set_config_value(name, value, config_type):
+ """Set a PassContext configuration value according to its value"""
+
+ if config_type == "IntImm":
+ # "Bool" configurations in the PassContext are recognized as
+ # IntImm, so deal with this case here
+ mapping_values = {
+ "false": False,
+ "true": True,
+ }
+
+ if value.isdigit():
+ parsed_value = int(value)
+ else:
+ # if not an int, accept only values on the mapping table, case insensitive
+ parsed_value = mapping_values.get(value.lower(), None)
+
+ if parsed_value is None:
+ raise TVMCException(f"Invalid value '{value}' for configuration '{name}'. ")
+
+ if config_type == "runtime.String":
+ parsed_value = value
+
+ return parsed_value
+
+
+def parse_configs(input_configs):
+ """Parse configuration values set via command line.
+
+ Parameters
+ ----------
+ input_configs: list of str
+ list of configurations provided via command line.
+
+ Returns
+ -------
+ pass_context_configs: dict
+ a dict containing key-value configs to be used in the PassContext.
+ """
+ all_configs = tvm.ir.transform.PassContext.list_configs()
+ supported_config_types = ("IntImm", "runtime.String")
+ supported_configs = [
+ name for name in all_configs.keys() if all_configs[name]["type"] in supported_config_types
+ ]
+ pass_context_configs = {}
+
+ if not input_configs:
+ return {}
+
+ for config in input_configs:
+ if len(config) == 0:
Review comment:
```suggestion
if not config:
```
##########
File path: python/tvm/driver/tvmc/compiler.py
##########
@@ -42,6 +42,13 @@ def add_compile_parser(subparsers):
parser = subparsers.add_parser("compile", help="compile a model.")
parser.set_defaults(func=drive_compile)
+ parser.add_argument(
+ "--config",
Review comment:
Would `build-config` or `pass-config` more intuitive?
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -415,3 +415,86 @@ def parse_shape_string(inputs_string):
shape_dict[name] = shape
return shape_dict
+
+
+def set_config_value(name, value, config_type):
+ """Set a PassContext configuration value according to its value"""
+
+ if config_type == "IntImm":
+ # "Bool" configurations in the PassContext are recognized as
+ # IntImm, so deal with this case here
+ mapping_values = {
+ "false": False,
+ "true": True,
+ }
+
+ if value.isdigit():
+ parsed_value = int(value)
+ else:
+ # if not an int, accept only values on the mapping table, case insensitive
+ parsed_value = mapping_values.get(value.lower(), None)
+
+ if parsed_value is None:
+ raise TVMCException(f"Invalid value '{value}' for configuration '{name}'. ")
+
+ if config_type == "runtime.String":
+ parsed_value = value
Review comment:
```suggestion
parsed_value = value
if config_type == "IntImm":
if value.isdigit():
parsed_value = int(value)
else:
# must be boolean values if not an int
try:
parsed_value = bool(distutils.util.strtobool(value))
except ValueError as err:
raise TVMCException(f"Invalid value '{value}' for configuration '{name}'. ")
```
If the dependency of `distuilts` is a concen, the following also works:
```
try:
parsed_value = json.loads(value)
except json.decoder.JSONDecodeError as err:
...
```
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -415,3 +415,86 @@ def parse_shape_string(inputs_string):
shape_dict[name] = shape
return shape_dict
+
+
+def set_config_value(name, value, config_type):
+ """Set a PassContext configuration value according to its value"""
+
+ if config_type == "IntImm":
+ # "Bool" configurations in the PassContext are recognized as
+ # IntImm, so deal with this case here
+ mapping_values = {
+ "false": False,
+ "true": True,
+ }
+
+ if value.isdigit():
+ parsed_value = int(value)
+ else:
+ # if not an int, accept only values on the mapping table, case insensitive
+ parsed_value = mapping_values.get(value.lower(), None)
+
+ if parsed_value is None:
+ raise TVMCException(f"Invalid value '{value}' for configuration '{name}'. ")
+
+ if config_type == "runtime.String":
+ parsed_value = value
+
+ return parsed_value
+
+
+def parse_configs(input_configs):
+ """Parse configuration values set via command line.
+
+ Parameters
+ ----------
+ input_configs: list of str
+ list of configurations provided via command line.
+
+ Returns
+ -------
+ pass_context_configs: dict
+ a dict containing key-value configs to be used in the PassContext.
+ """
+ all_configs = tvm.ir.transform.PassContext.list_configs()
+ supported_config_types = ("IntImm", "runtime.String")
+ supported_configs = [
+ name for name in all_configs.keys() if all_configs[name]["type"] in supported_config_types
+ ]
+ pass_context_configs = {}
+
+ if not input_configs:
+ return {}
+
+ for config in input_configs:
+ if len(config) == 0:
+ raise TVMCException(
+ f"Invalid format for configuration '{config}', use <config>=<value>"
+ )
+
+ # Each config is expected to be provided as "name=value"
+ try:
+ name, value = config.split("=")
+ name = name.strip()
+ value = value.strip()
+ except ValueError:
+ raise TVMCException(
+ f"Invalid format for configuration '{config}', use <config>=<value>"
+ )
+
+ if name not in all_configs:
+ raise TVMCException(
+ f"Configuration '{name}' is not defined in TVM. "
+ f"These are the existing configurations: {', '.join(all_configs)}"
+ )
+
+ if name not in supported_configs:
+ raise TVMCException(
+ f"Configuration '{name}' is not supported in TVMC. "
Review comment:
Better to explain the reason (i.e., not supported config value type).
--
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] [tvm] leandron commented on a change in pull request #8253: [tvmc] Add a --config option to `tvmc compile`
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #8253:
URL: https://github.com/apache/tvm/pull/8253#discussion_r651581339
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -415,3 +415,86 @@ def parse_shape_string(inputs_string):
shape_dict[name] = shape
return shape_dict
+
+
+def set_config_value(name, value, config_type):
+ """Set a PassContext configuration value according to its value"""
+
+ if config_type == "IntImm":
+ # "Bool" configurations in the PassContext are recognized as
+ # IntImm, so deal with this case here
+ mapping_values = {
+ "false": False,
+ "true": True,
+ }
+
+ if value.isdigit():
+ parsed_value = int(value)
+ else:
+ # if not an int, accept only values on the mapping table, case insensitive
+ parsed_value = mapping_values.get(value.lower(), None)
+
+ if parsed_value is None:
+ raise TVMCException(f"Invalid value '{value}' for configuration '{name}'. ")
+
+ if config_type == "runtime.String":
+ parsed_value = value
+
+ return parsed_value
+
+
+def parse_configs(input_configs):
+ """Parse configuration values set via command line.
+
+ Parameters
+ ----------
+ input_configs: list of str
+ list of configurations provided via command line.
+
+ Returns
+ -------
+ pass_context_configs: dict
+ a dict containing key-value configs to be used in the PassContext.
+ """
+ all_configs = tvm.ir.transform.PassContext.list_configs()
+ supported_config_types = ("IntImm", "runtime.String")
+ supported_configs = [
+ name for name in all_configs.keys() if all_configs[name]["type"] in supported_config_types
+ ]
+ pass_context_configs = {}
+
+ if not input_configs:
+ return {}
Review comment:
Done.
--
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] [tvm] leandron commented on a change in pull request #8253: [tvmc] Add a --config option to `tvmc compile`
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #8253:
URL: https://github.com/apache/tvm/pull/8253#discussion_r651584886
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -415,3 +415,86 @@ def parse_shape_string(inputs_string):
shape_dict[name] = shape
return shape_dict
+
+
+def set_config_value(name, value, config_type):
+ """Set a PassContext configuration value according to its value"""
+
+ if config_type == "IntImm":
+ # "Bool" configurations in the PassContext are recognized as
+ # IntImm, so deal with this case here
+ mapping_values = {
+ "false": False,
+ "true": True,
+ }
+
+ if value.isdigit():
+ parsed_value = int(value)
+ else:
+ # if not an int, accept only values on the mapping table, case insensitive
+ parsed_value = mapping_values.get(value.lower(), None)
+
+ if parsed_value is None:
+ raise TVMCException(f"Invalid value '{value}' for configuration '{name}'. ")
+
+ if config_type == "runtime.String":
+ parsed_value = value
+
+ return parsed_value
+
+
+def parse_configs(input_configs):
+ """Parse configuration values set via command line.
+
+ Parameters
+ ----------
+ input_configs: list of str
+ list of configurations provided via command line.
+
+ Returns
+ -------
+ pass_context_configs: dict
+ a dict containing key-value configs to be used in the PassContext.
+ """
+ all_configs = tvm.ir.transform.PassContext.list_configs()
+ supported_config_types = ("IntImm", "runtime.String")
+ supported_configs = [
+ name for name in all_configs.keys() if all_configs[name]["type"] in supported_config_types
+ ]
+ pass_context_configs = {}
+
+ if not input_configs:
+ return {}
+
+ for config in input_configs:
+ if len(config) == 0:
+ raise TVMCException(
+ f"Invalid format for configuration '{config}', use <config>=<value>"
+ )
+
+ # Each config is expected to be provided as "name=value"
+ try:
+ name, value = config.split("=")
+ name = name.strip()
+ value = value.strip()
+ except ValueError:
+ raise TVMCException(
+ f"Invalid format for configuration '{config}', use <config>=<value>"
+ )
+
+ if name not in all_configs:
+ raise TVMCException(
+ f"Configuration '{name}' is not defined in TVM. "
+ f"These are the existing configurations: {', '.join(all_configs)}"
+ )
+
+ if name not in supported_configs:
+ raise TVMCException(
+ f"Configuration '{name}' is not supported in TVMC. "
Review comment:
Done
--
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] [tvm] gromero commented on a change in pull request #8253: [tvmc] Add a --config option to `tvmc compile`
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #8253:
URL: https://github.com/apache/tvm/pull/8253#discussion_r651373115
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -415,3 +415,86 @@ def parse_shape_string(inputs_string):
shape_dict[name] = shape
return shape_dict
+
+
+def set_config_value(name, value, config_type):
Review comment:
Just a semantic nit here: it seems it's more a "get" than a "set" function? Like `get_pass_config_value` (also taking into account the suggestion from @comaniac about using `pass-config` flag instead of `config`, which I liked :)
##########
File path: python/tvm/driver/tvmc/compiler.py
##########
@@ -42,6 +42,13 @@ def add_compile_parser(subparsers):
parser = subparsers.add_parser("compile", help="compile a model.")
parser.set_defaults(func=drive_compile)
+ parser.add_argument(
+ "--config",
+ action="append",
+ metavar=("name=value"),
+ help="configurations to be used at compile time. A subset of options provided "
+ "by TVM are supported. e.g. 'relay.backend.use_auto_scheduler=0'",
Review comment:
I'm wondering if it would make sense to enhance the help message a bit more so users don't try to do something like:
`--config="tir.disable_vectorize=true,tir.disable_assert=true"` instead of
`--config=tir.disable_vectorize=true --config=tir.disable_assert=true"`, i.e. know easily that multiple `--config` flags can be used and will be appended.
I also see duplicated and even conflicting flags don't generate any error or warning. Should we treat them too? Like:
```
$ python3 . compile --target="llvm" --config "tir.disable_vectorize=true" --config "tir.disable_vectorize=false" --config "tir.disable_assert=true" ./sine_model.tflite
One or more operators have not been tuned. Please tune your model for better performance. Use DEBUG logging level to see more details.
$
```
and
```
$ python3 . compile --target="llvm" --config "tir.disable_vectorize=true" --config "tir.disable_vectorize=true" --config "tir.disable_assert=true" ./sine_model.tflite
One or more operators have not been tuned. Please tune your model for better performance. Use DEBUG logging level to see more details.
$
```
##########
File path: tests/python/driver/tvmc/test_tvmc_common.py
##########
@@ -306,3 +306,49 @@ def test_parse_quotes_and_separators_on_options():
assert len(targets_double_quote) == 1
assert "+v1.0x,+value" == targets_double_quote[0]["opts"]["option1"]
+
+
+def test_config_invalid_format():
+ with pytest.raises(TVMCException):
+ _ = tvmc.common.parse_configs(["relay.backend.use_auto_scheduler.missing.value"])
+
+
+def test_config_missing_from_tvm():
+ with pytest.raises(TVMCException):
+ _ = tvmc.common.parse_configs(["relay.backend.use_auto_scheduler.missing.value=1234"])
+
+
+def test_config_unsupported_tvmc_config():
+ with pytest.raises(TVMCException):
+ _ = tvmc.common.parse_configs(["tir.LoopPartition=value"])
+
+
+def test_config_empty():
+ with pytest.raises(TVMCException):
+ _ = tvmc.common.parse_configs([""])
+
+
+def test_config_valid_config_bool():
+ configs = tvmc.common.parse_configs(["relay.backend.use_auto_scheduler=true"])
+
+ assert len(configs) == 1
+ assert "relay.backend.use_auto_scheduler" in configs.keys()
+ assert configs["relay.backend.use_auto_scheduler"] == True
+
+
+def test_config_valid_multiple_configs():
+ configs = tvmc.common.parse_configs(
+ [
+ "relay.backend.use_auto_scheduler=false",
+ "tir.detect_global_barrier=10",
+ "relay.ext.vitis_ai.options.build_dir=mystring",
Review comment:
CI complains about it. I believe `relay.ext.vitis_ai.options.build_dir` is neither `IntImm` nor `runtime.String` type?
--
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] [tvm] leandron commented on a change in pull request #8253: [tvmc] Add a --config option to `tvmc compile`
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #8253:
URL: https://github.com/apache/tvm/pull/8253#discussion_r651680618
##########
File path: python/tvm/driver/tvmc/compiler.py
##########
@@ -42,6 +42,13 @@ def add_compile_parser(subparsers):
parser = subparsers.add_parser("compile", help="compile a model.")
parser.set_defaults(func=drive_compile)
+ parser.add_argument(
+ "--config",
+ action="append",
+ metavar=("name=value"),
+ help="configurations to be used at compile time. A subset of options provided "
+ "by TVM are supported. e.g. 'relay.backend.use_auto_scheduler=0'",
Review comment:
> I'm wondering if it would make sense to enhance the help message a bit more so users don't try to do something like:
Fixed.
>
> I also see duplicated and even conflicting flags don't generate any error or warning. Should we treat them too? Like:
>
I think most tools won't complain if you provide repeated configs. In this case, similar to what most tools e.g. _docker, bash_, will just assume the latest (considering parsing is done left to right) value is the one to be used.
note: In some other cases, like in the `--target`, I'm validating duplicates because I need to translate that plain string to various internal APIs.
--
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] [tvm] leandron commented on pull request #8253: [tvmc] Add a --config option to `tvmc compile`
Posted by GitBox <gi...@apache.org>.
leandron commented on pull request #8253:
URL: https://github.com/apache/tvm/pull/8253#issuecomment-861832097
@comaniac @gromero I updated this incorporating most of you comments. Please have a look when you have a moment.
--
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