[tvmc] Add a --config option to tvmc compile
#8253
Conversation
| 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 |
There was a problem hiding this comment.
| 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 | |
| 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:
...
There was a problem hiding this 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.
There was a problem hiding this 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.
* 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
| 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 |
There was a problem hiding this 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.
|
just curious if there will be a disk format as well so that people don't have to maintain long command-lines? |
* 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
* 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
4 participants
[tvmc] Add a
--configoption totvmc compile:PassContextvia command lineThis builds on top of #8212 and #8226, in which I exposed such API parts to the Python layer.
cc @gromero @comaniac @manupa-arm