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/10/08 02:56:54 UTC
[GitHub] [tvm] gromero opened a new pull request #9229: [TVMC] Add new micro context
gromero opened a new pull request #9229:
URL: https://github.com/apache/tvm/pull/9229
This patchset adds a new micro context to TVMC under `tvmc micro` command, allowing `tvmc` to be used for compiling, build, flashing, and running models on microTVM target devices.
The following typical workflow is provided as an example:
```bash
$ wget https://github.com/tensorflow/tflite-micro/raw/main/tensorflow/lite/micro/examples/micro_speech/micro_speech.tflite
$ tvmc compile ./micro_speech.tflite --target="c -keys=cpu -link-params=0 -march=armv7e-m -mcpu=cortex-m7 -model=stm32f746xx -runtime=c -system-lib=1" --output micro_speech.tar --output-format mlf --pass-config tir.disable_vectorize=1 --disabled-pass="AlterOpLayout"
$ tvmc micro create-project /tmp/micro_speech ./micro_speech.tar zephyr -o project_type=host_driven zephyr_board=stm32f746g_disco
$ tvmc micro build /tmp/micro_speech zephyr -o zephyr_board=stm32f746g_disco
$ tvmc micro flash /tmp/micro_speech zephyr -o zephyr_board=stm32f746g_disco
$ tvmc run --device micro /tmp/micro_speech --options zephyr_board=stm32f746g_disco --print-top 6 --fill-mode random
MLF found, using micro target runner.
[[ 3 2 1 0]
[ 35 -63 -100 -128]]
```
A Pre-RFC will be published soon associated to this patchset to start a discussion about that new context in TVMC.
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r738593504
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -384,13 +482,27 @@ def run_module(
else:
logger.debug("Running on remote RPC with no key.")
session = rpc.connect(hostname, port)
+ elif device == "micro":
+ # Remote RPC (running on a micro target)
+ logger.debug("Running on remote RPC (micro target).")
+ try:
+ with ExitStack() as stack:
Review comment:
oh got it. thanks, I misunderstood the purpose of `contextlib` the first time.
So, I've put all below line 475 inside the `with` scope as you suggested. Just a note that was precisely something like that I was trying to avoid since the beginning, that's why I thought of introducing `open()` and `close()` methods at first then use them explicitly in `runner.py`, first when opening the transport and creating a session, than later at end of `run_module`, checking again if `device == "micro"` and if so calling `close()` explicitly. But yeah If you think that putting everything after line 475 inside `with` is ok, alright then.
--
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] gromero removed a comment on pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero removed a comment on pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#issuecomment-954735900
> Please add `[microtvm]` to PR title.
I think it's ok to have two tags (
--
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] gromero commented on pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#issuecomment-951140008
@areusch @mehrdadh Thanks a lot for the reviews. PTAL.
I think there are only two major issues remaining to get addressed:
1. https://github.com/apache/tvm/pull/9229#discussion_r727538431
2. https://github.com/apache/tvm/pull/9229#discussion_r727551231
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735799995
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,278 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+from .fmtopt import format_option
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
+ARDUINO_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/arduino/template_project"
+
+
+TEMPLATES = {
+ "zephyr": ZEPHYR_TEMPLATE_DIR,
+ "arduino": ARDUINO_TEMPLATE_DIR,
+}
+
+
+@register_parser
+def add_micro_parser(subparsers, main_parser):
+ micro = subparsers.add_parser("micro", help="select micro context.")
+ micro.set_defaults(func=drive_micro)
+
+ micro_parser = micro.add_subparsers(title="subcommands")
+ # Selecting a subcommand under 'micro' is mandatory
+ micro_parser.required = True
+ micro_parser.dest = "subcommand" # options available to select
+
+ # 'create_project' subcommand
+ create_project_parser = micro_parser.add_parser(
+ "create-project", help="create a project template of a given type or given a template dir."
+ )
+ create_project_parser.set_defaults(subcommand_handler=create_project_handler)
+ create_project_parser.add_argument(
+ "PROJECT_DIR",
+ help="Project dir where the new project based on the template dir will be created.",
+ )
+ create_project_parser.add_argument("MLF", help="MLF .tar archive.")
+ create_project_parser.add_argument(
+ "-f",
+ "--force",
+ action="store_true",
+ help="force project creating even if the specified PROJECT_DIR already exists.",
+ )
+
+ # 'build' subcommand
+ build_parser = micro_parser.add_parser(
+ "build",
+ help="build a project dir, generally creating an image to be flashed, e.g. zephyr.elf.",
+ )
+ build_parser.set_defaults(subcommand_handler=build_handler)
+ build_parser.add_argument("PROJECT_DIR", help="Project dir to build.")
+ build_parser.add_argument("-f", "--force", action="store_true", help="Force rebuild.")
+
+ # 'flash' subcommand
+ flash_parser = micro_parser.add_parser(
+ "flash", help="flash the built image on a given micro target."
+ )
+ flash_parser.set_defaults(subcommand_handler=flash_handler)
+ flash_parser.add_argument("PROJECT_DIR", help="Project dir with a image built.")
+
+ # TODO(gromero): list and select serial when multiple devices exist
+ # It's not clear yet if the device list should be determined by TVMC or returned by the Project API
Review comment:
@mehrdadh This a leftover from the first implementation sorry. I think that listing the available serial number for specific project options must be a duty for the Project API, which in the end is where the code for talking to the device is. Otherwise it would probably result in duplicated code, besides being very hard to maintain in TVMC because a project option that accepts a serial number can virtually be addded to in any `tvmc` subcommand, so it would kind defeat the automatic option detection mechanism here.
--
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] gromero commented on a change in pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r747902235
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -38,6 +41,36 @@
class TVMCException(Exception):
"""TVMC Exception"""
+class TVMCSilentArgumentParser(argparse.ArgumentParser):
Review comment:
If you're ok now I'll rebase that branch on main and promote it to a PR so the CI can kick in.
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735738703
##########
File path: python/tvm/driver/tvmc/fmtopt.py
##########
@@ -0,0 +1,70 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from textwrap import TextWrapper
+
+
+def format_option(option_text, help_text, required=True):
+ # Maximum column length starting at the option name.
+ COL_LEN = 80
+
+ # Extract option name (option name + '=' char, i.e. '=' is part of what
+ # is considered here 'optname') plus its length for choices and help
+ # text justification.
+ optname_break = option_text.find("=")
+ optname = option_text[: optname_break + 1] # '=' is part of the option name in this context
+ optname_len = len(optname)
+
+ # Prepare optname + choices text chunck.
+ choices_text = option_text[optname_break + 1 :] # '=' is not part of the choices
+ wrapper = TextWrapper(width=COL_LEN - optname_len)
+ choices_lines = wrapper.wrap(choices_text)
+
+ # Set first choices line which merely appends to optname string.
+ # No justification is necessary for the first line since first
+ # line was wrapped based on COL_LEN - optname_len, i.e. considering
+ # optname_len.
+ choices_just_lines = [optname + choices_lines[0]]
+
+ # Justify the remaining lines based on first optname + '='.
+ for line in choices_lines[1:]:
+ line_len = len(line)
+ line_just = line.rjust(
+ optname_len + 1 + line_len
+ ) # add 1 to align after '{' in the line above
+ choices_just_lines.append(line_just)
+
+ choices_text_just_chunk = "\n".join(choices_just_lines)
+
+ # Prepare help text chunck.
+ wrapper = TextWrapper(width=120)
Review comment:
Initial idea was to wrap also the help text line (not just option choices) and justify it so it would be below the optname/choices justified text lines. But later I realized that it would look odd when it was displayed with other non-format help text directly added by `argparse`. So I picked up 120 cause it was an adhoc value that would cause no line wrapping...
I've refactor a bit the code so now there is a separate module parameter `MAX_HELP_TEXT_COL_LEN` (https://github.com/apache/tvm/pull/9229/commits/287b643c4c1f37620f32fae1fa242af5e99df057#diff-829b6ec83c95abdda9185a08f28756b93c96e4a5b2b66ce502ee3cb43e8918b7R28) that can be used to control the help text line wrapping and it's currently set to `0`, which means it's disabled.
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735739453
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,278 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+from .fmtopt import format_option
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
+ARDUINO_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/arduino/template_project"
+
+
+TEMPLATES = {
+ "zephyr": ZEPHYR_TEMPLATE_DIR,
+ "arduino": ARDUINO_TEMPLATE_DIR,
+}
+
+
+@register_parser
+def add_micro_parser(subparsers, main_parser):
+ micro = subparsers.add_parser("micro", help="select micro context.")
+ micro.set_defaults(func=drive_micro)
+
+ micro_parser = micro.add_subparsers(title="subcommands")
+ # Selecting a subcommand under 'micro' is mandatory
+ micro_parser.required = True
+ micro_parser.dest = "subcommand" # options available to select
+
+ # 'create_project' subcommand
+ create_project_parser = micro_parser.add_parser(
+ "create-project", help="create a project template of a given type or given a template dir."
+ )
+ create_project_parser.set_defaults(subcommand_handler=create_project_handler)
+ create_project_parser.add_argument(
+ "PROJECT_DIR",
+ help="Project dir where the new project based on the template dir will be created.",
+ )
+ create_project_parser.add_argument("MLF", help="MLF .tar archive.")
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735792710
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -520,3 +522,84 @@ def parse_configs(input_configs):
pass_context_configs[name] = parsed_value
return pass_context_configs
+
+
+def get_project_options(project_info):
+ options = project_info["project_options"]
+
+ options_by_method = defaultdict(list)
+ for opt in options:
+ # Get list of methods associated with an option based on the
+ # existance of a 'required' or 'optional' lists. API specification
+ # guarantees at least one of these lists will exist. If a list does
+ # not exist it's returned as None by the API.
+ metadata = ["required", "optional"]
+ om = [(opt[md], True if md == "required" else False) for md in metadata if opt[md]]
+ for methods, is_opt_required in om:
+ for method in methods:
+ name = opt["name"]
+
+ if opt["type"] == "bool":
+ # TODO(gromero): Use only choices= and merge with non-bool options below
+ option_choices_text = f"{name}={{true, false}}"
+
+ else:
+ if opt["choices"]:
+ choices = "{" + ", ".join(opt["choices"]) + "}"
+ else:
+ choices = opt["name"].upper()
+
+ option_choices_text = f"{name}={choices}"
+
+ help_text = opt["help"][0].lower() + opt["help"][1:]
+ formatted_help_text = format_option(option_choices_text, help_text, is_opt_required)
+
+ option = {
+ "name": opt["name"],
+ "choices": opt["choices"],
+ "help_text": formatted_help_text,
+ "required": is_opt_required,
+ }
+ options_by_method[method].append(option)
+
+ return options_by_method
+
+
+def get_options(options):
Review comment:
@mehrdadh Fixed ZEPHYR_BASE as you suggested in https://github.com/apache/tvm/pull/9229/commits/efbc2803a2f5c4775a4d6c0019e3ce0d3e262718#diff-e9070632c64e33062450a5c887a7a6b683d7294064584407aeff7aef24031dc1R280
--
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] gromero commented on a change in pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r749832951
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -109,21 +168,58 @@ def drive_run(args):
Arguments from command line parser.
"""
- rpc_hostname, rpc_port = common.tracker_host_port_from_cli(args.rpc_tracker)
+ path = pathlib.Path(args.PATH)
+ options = None
+ if args.device == "micro":
+ path = path / "model.tar"
Review comment:
hmr, I see. Yeah I'll prepare a follow-on PR to fix it. :+1:
--
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] mehrdadh commented on a change in pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r740393544
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,277 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
Review comment:
https://github.com/apache/tvm/pull/9309 is merged 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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735760958
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -97,7 +111,55 @@ def add_run_parser(subparsers):
help="hostname (required) and port (optional, defaults to 9090) of the RPC tracker, "
"e.g. '192.168.0.100:9999'",
)
- parser.add_argument("FILE", help="path to the compiled module file")
+ parser.add_argument(
+ "PATH",
+ help="path to the compiled module file or to the project directory (micro devices only).",
+ )
+
+ #
+ # Hack to lookahead if 'run' was selected and make
+ # two dynamic parsers (in micro.py and in runner.py) work.
+ #
+ # print(sys.argv)
+ if "run" in sys.argv:
Review comment:
That is a tricky one. The tricky thing here is that due to tvmc design - that uses a decorator to build a list of functions that will later be called explicitly to augment the parser - a call to `parser_known_args()` can not exit when the command line doesn't match, i.e. when a command line doesn't even satisfies the minimum required syntax so `parser_known_args()` can return, so it exists and can't even provide a known/unknown list of options.
If `parser_known_args()` returns to early there won't be a chance of calling the second function responsible to build the second dynamic parser (in this case parser in micro.py will be called secondly, as per import order in `__init__.py`). So when the first call to `parser_known_args()` returns and prints the options missing it will miss the options that would be included by the second function responsible to build the second dynamic parser.
The case you've asked about ("what if `run` is e.g. the value of --template-dir?") nicely works:
```
gromero@amd:~/git/tvm$ tvmc micro create-project /tmp/x16 ~/scripts/sine.tar template --template-dir ./apps/microtvm/run_adhoc_template/template_project --project-option zephyr_board=stm32f746g_disco project_type=host_driven
gromero@amd:~/git/tvm$ ls -l /tmp/x16
total 108
-rw-rw-r-- 1 gromero gromero 1404 Oct 25 14:16 boards.json
-rw-rw-r-- 1 gromero gromero 2245 Oct 25 14:30 CMakeLists.txt
drwxrwxr-x 4 gromero gromero 4096 Oct 25 14:30 crt
drwxrwxr-x 2 gromero gromero 4096 Oct 25 14:30 crt_config
-rw-rw-r-- 1 gromero gromero 25434 Oct 25 14:16 microtvm_api_server.py
drwx------ 6 gromero gromero 4096 Jul 27 20:07 model
-rw-rw-r-- 1 gromero gromero 51200 Jul 27 20:07 model.tar
-rw-rw-r-- 1 gromero gromero 408 Oct 25 14:30 prj.conf
drwxrwxr-x 2 gromero gromero 4096 Oct 25 14:16 src
gromero@amd:~/git/tvm$
```
But the corner case is actually something like:
```
gromero@amd:~/git/tvm$ tvmc micro create-project /tmp/x17 ~/scripts/sine.tar template --template-dir ./apps/microtvm/run_adhoc_templatez/template_project -x run
usage: tvmc [-h] [-v] [--version] {tune,compile,run} ...
tvmc: error: invalid choice: 'micro' (choose from 'tune', 'compile', 'run')
```
Note that 'micro' context is not understood because `parse_known_args()` returns before parser in `micro.py` has the chance to augment the parser and add the micro context. If I add the dynamic parser in `micro.py` the dynamic parser `runner.py` will suffer that "vanishment" instead.
However to me the corner case seems to be a very rare to happen, hence the "lookahead" hack.
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735731084
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -520,3 +522,84 @@ def parse_configs(input_configs):
pass_context_configs[name] = parsed_value
return pass_context_configs
+
+
+def get_project_options(project_info):
Review comment:
Sure. Done in https://github.com/apache/tvm/pull/9229/commits/303730a2e380922d968424b34517fdfc6f226b1a
--
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] mehrdadh commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r728480988
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -520,3 +522,84 @@ def parse_configs(input_configs):
pass_context_configs[name] = parsed_value
return pass_context_configs
+
+
+def get_project_options(project_info):
+ options = project_info["project_options"]
+
+ options_by_method = defaultdict(list)
+ for opt in options:
+ # Get list of methods associated with an option based on the
+ # existance of a 'required' or 'optional' lists. API specification
+ # guarantees at least one of these lists will exist. If a list does
+ # not exist it's returned as None by the API.
+ metadata = ["required", "optional"]
+ om = [(opt[md], True if md == "required" else False) for md in metadata if opt[md]]
+ for methods, is_opt_required in om:
+ for method in methods:
+ name = opt["name"]
+
+ if opt["type"] == "bool":
+ # TODO(gromero): Use only choices= and merge with non-bool options below
+ option_choices_text = f"{name}={{true, false}}"
+
+ else:
+ if opt["choices"]:
+ choices = "{" + ", ".join(opt["choices"]) + "}"
+ else:
+ choices = opt["name"].upper()
+
+ option_choices_text = f"{name}={choices}"
+
+ help_text = opt["help"][0].lower() + opt["help"][1:]
+ formatted_help_text = format_option(option_choices_text, help_text, is_opt_required)
+
+ option = {
+ "name": opt["name"],
+ "choices": opt["choices"],
+ "help_text": formatted_help_text,
+ "required": is_opt_required,
+ }
+ options_by_method[method].append(option)
+
+ return options_by_method
+
+
+def get_options(options):
Review comment:
Could you please add docstring to all methods? it makes it much easier to understand
also maybe add type to arguments?
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,278 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+from .fmtopt import format_option
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
+ARDUINO_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/arduino/template_project"
+
+
+TEMPLATES = {
+ "zephyr": ZEPHYR_TEMPLATE_DIR,
+ "arduino": ARDUINO_TEMPLATE_DIR,
+}
+
+
+@register_parser
+def add_micro_parser(subparsers, main_parser):
+ micro = subparsers.add_parser("micro", help="select micro context.")
+ micro.set_defaults(func=drive_micro)
+
+ micro_parser = micro.add_subparsers(title="subcommands")
+ # Selecting a subcommand under 'micro' is mandatory
+ micro_parser.required = True
+ micro_parser.dest = "subcommand" # options available to select
+
+ # 'create_project' subcommand
+ create_project_parser = micro_parser.add_parser(
+ "create-project", help="create a project template of a given type or given a template dir."
+ )
+ create_project_parser.set_defaults(subcommand_handler=create_project_handler)
+ create_project_parser.add_argument(
+ "PROJECT_DIR",
+ help="Project dir where the new project based on the template dir will be created.",
+ )
+ create_project_parser.add_argument("MLF", help="MLF .tar archive.")
+ create_project_parser.add_argument(
+ "-f",
+ "--force",
+ action="store_true",
+ help="force project creating even if the specified PROJECT_DIR already exists.",
+ )
+
+ # 'build' subcommand
+ build_parser = micro_parser.add_parser(
+ "build",
+ help="build a project dir, generally creating an image to be flashed, e.g. zephyr.elf.",
+ )
+ build_parser.set_defaults(subcommand_handler=build_handler)
+ build_parser.add_argument("PROJECT_DIR", help="Project dir to build.")
+ build_parser.add_argument("-f", "--force", action="store_true", help="Force rebuild.")
+
+ # 'flash' subcommand
+ flash_parser = micro_parser.add_parser(
+ "flash", help="flash the built image on a given micro target."
+ )
+ flash_parser.set_defaults(subcommand_handler=flash_handler)
+ flash_parser.add_argument("PROJECT_DIR", help="Project dir with a image built.")
+
+ # TODO(gromero): list and select serial when multiple devices exist
+ # It's not clear yet if the device list should be determined by TVMC or returned by the Project API
Review comment:
I think serial number should come from TVMC. It could be optional and if it's not passed used the one that project API finds.
##########
File path: apps/microtvm/zephyr/template_project/microtvm_api_server.py
##########
@@ -229,40 +229,69 @@ def _get_nrf_device_args(options):
PROJECT_OPTIONS = [
server.ProjectOption(
"extra_files_tar",
+ optional=["generate_project"],
+ type="str",
help="If given, during generate_project, uncompress the tarball at this path into the project dir.",
),
server.ProjectOption(
- "gdbserver_port", help=("If given, port number to use when running the local gdbserver.")
+ "gdbserver_port",
+ help=("If given, port number to use when running the local gdbserver."),
+ optional=["open_transport"],
+ type="int",
),
server.ProjectOption(
"nrfjprog_snr",
+ optional=["open_transport"],
+ type="int",
help=("When used with nRF targets, serial # of the attached board to use, from nrfjprog."),
),
server.ProjectOption(
"openocd_serial",
+ optional=["open_transport"],
+ type="int",
help=("When used with OpenOCD targets, serial # of the attached board to use."),
),
server.ProjectOption(
"project_type",
- help="Type of project to generate.",
choices=tuple(PROJECT_TYPES),
+ required=["generate_project"],
+ type="str",
+ help="Type of project to generate.",
+ ),
+ server.ProjectOption(
+ "verbose",
+ optional=["build"],
+ help="Run build with verbose output.",
+ type="bool",
),
- server.ProjectOption("verbose", help="Run build with verbose output.", choices=(True, False)),
server.ProjectOption(
"west_cmd",
+ optional=["generate_project"],
+ default="python3 -m west",
+ type="str",
help=(
"Path to the west tool. If given, supersedes both the zephyr_base "
"option and ZEPHYR_BASE environment variable."
),
),
- server.ProjectOption("zephyr_base", help="Path to the zephyr base directory."),
+ server.ProjectOption(
+ "zephyr_base",
+ optional=["build", "open_transport"],
+ default="ZEPHYR_BASE",
Review comment:
I think by default we could get it from environment variable ZEPHYR_BASE?
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,278 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+from .fmtopt import format_option
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
+ARDUINO_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/arduino/template_project"
+
+
+TEMPLATES = {
+ "zephyr": ZEPHYR_TEMPLATE_DIR,
+ "arduino": ARDUINO_TEMPLATE_DIR,
+}
+
+
+@register_parser
+def add_micro_parser(subparsers, main_parser):
+ micro = subparsers.add_parser("micro", help="select micro context.")
+ micro.set_defaults(func=drive_micro)
+
+ micro_parser = micro.add_subparsers(title="subcommands")
+ # Selecting a subcommand under 'micro' is mandatory
+ micro_parser.required = True
+ micro_parser.dest = "subcommand" # options available to select
+
+ # 'create_project' subcommand
+ create_project_parser = micro_parser.add_parser(
+ "create-project", help="create a project template of a given type or given a template dir."
+ )
+ create_project_parser.set_defaults(subcommand_handler=create_project_handler)
+ create_project_parser.add_argument(
+ "PROJECT_DIR",
Review comment:
+1 to lowercase.
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735800857
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -366,13 +460,17 @@ def run_module(
"Try calling tvmc.compile on the model before running it."
)
- # Currently only two package formats are supported: "classic" and
- # "mlf". The later can only be used for micro targets, i.e. with microTVM.
- if tvmc_package.type == "mlf":
- raise TVMCException(
- "You're trying to run a model saved using the Model Library Format (MLF)."
- "MLF can only be used to run micro targets (microTVM)."
- )
+ micro = False
Review comment:
Done in https://github.com/apache/tvm/pull/9229/commits/c64d0952b6f95b984fd9eba93abb351be179c50a
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735780219
##########
File path: python/tvm/micro/session.py
##########
@@ -107,7 +107,7 @@ def _wrap_transport_write(self, data, timeout_microsec):
return len(data) # TODO(areusch): delete
- def __enter__(self):
+ def open(self):
Review comment:
hmr actually I would like to add also the possibility to let the user decide which mechanism he/she would like to use, with `with` (context manager) or w/o a context manage, calling `open` and `close.
But accordingly to what we've discussed in the past I revered this change and used `contextlib` to manage the context.
Reverted in: https://github.com/apache/tvm/pull/9229/commits/42167f90a5b32055fad7b4f50d3f78e37e8dd15b
Kept the fix for the typo in: https://github.com/apache/tvm/pull/9229/commits/bf94efe21f2045bd5933dcb98995ae7d4c0430f5
Use of contextlib in: https://github.com/apache/tvm/pull/9229/commits/874382e6786d3996527d19e4002ee23d8e6c7c9f
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735730452
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -520,3 +522,84 @@ def parse_configs(input_configs):
pass_context_configs[name] = parsed_value
return pass_context_configs
+
+
+def get_project_options(project_info):
+ options = project_info["project_options"]
+
+ options_by_method = defaultdict(list)
+ for opt in options:
+ # Get list of methods associated with an option based on the
+ # existance of a 'required' or 'optional' lists. API specification
+ # guarantees at least one of these lists will exist. If a list does
+ # not exist it's returned as None by the API.
+ metadata = ["required", "optional"]
+ om = [(opt[md], True if md == "required" else False) for md in metadata if opt[md]]
+ for methods, is_opt_required in om:
+ for method in methods:
+ name = opt["name"]
+
+ if opt["type"] == "bool":
+ # TODO(gromero): Use only choices= and merge with non-bool options below
+ option_choices_text = f"{name}={{true, false}}"
Review comment:
Done in https://github.com/apache/tvm/pull/9229/commits/caf01beed79c6dacf67252d518117ffd53fa37b6
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735728910
##########
File path: apps/microtvm/zephyr/template_project/microtvm_api_server.py
##########
@@ -229,40 +229,69 @@ def _get_nrf_device_args(options):
PROJECT_OPTIONS = [
server.ProjectOption(
"extra_files_tar",
+ optional=["generate_project"],
+ type="str",
help="If given, during generate_project, uncompress the tarball at this path into the project dir.",
),
server.ProjectOption(
- "gdbserver_port", help=("If given, port number to use when running the local gdbserver.")
+ "gdbserver_port",
+ help=("If given, port number to use when running the local gdbserver."),
+ optional=["open_transport"],
+ type="int",
),
server.ProjectOption(
"nrfjprog_snr",
+ optional=["open_transport"],
+ type="int",
help=("When used with nRF targets, serial # of the attached board to use, from nrfjprog."),
),
server.ProjectOption(
"openocd_serial",
+ optional=["open_transport"],
+ type="int",
help=("When used with OpenOCD targets, serial # of the attached board to use."),
),
server.ProjectOption(
"project_type",
- help="Type of project to generate.",
choices=tuple(PROJECT_TYPES),
+ required=["generate_project"],
+ type="str",
+ help="Type of project to generate.",
+ ),
+ server.ProjectOption(
+ "verbose",
+ optional=["build"],
+ help="Run build with verbose output.",
+ type="bool",
),
- server.ProjectOption("verbose", help="Run build with verbose output.", choices=(True, False)),
server.ProjectOption(
"west_cmd",
+ optional=["generate_project"],
+ default="python3 -m west",
+ type="str",
help=(
"Path to the west tool. If given, supersedes both the zephyr_base "
"option and ZEPHYR_BASE environment variable."
),
),
- server.ProjectOption("zephyr_base", help="Path to the zephyr base directory."),
+ server.ProjectOption(
+ "zephyr_base",
+ optional=["build", "open_transport"],
+ default="ZEPHYR_BASE",
Review comment:
@areusch It was actually wrong. I've fixed as suggested by @mehrdadh . Done in https://github.com/apache/tvm/pull/9229/commits/efbc2803a2f5c4775a4d6c0019e3ce0d3e262718#diff-e9070632c64e33062450a5c887a7a6b683d7294064584407aeff7aef24031dc1R280
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735742683
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,278 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+from .fmtopt import format_option
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
+ARDUINO_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/arduino/template_project"
+
+
+TEMPLATES = {
+ "zephyr": ZEPHYR_TEMPLATE_DIR,
+ "arduino": ARDUINO_TEMPLATE_DIR,
+}
+
+
+@register_parser
+def add_micro_parser(subparsers, main_parser):
+ micro = subparsers.add_parser("micro", help="select micro context.")
+ micro.set_defaults(func=drive_micro)
+
+ micro_parser = micro.add_subparsers(title="subcommands")
+ # Selecting a subcommand under 'micro' is mandatory
+ micro_parser.required = True
+ micro_parser.dest = "subcommand" # options available to select
+
+ # 'create_project' subcommand
+ create_project_parser = micro_parser.add_parser(
+ "create-project", help="create a project template of a given type or given a template dir."
+ )
+ create_project_parser.set_defaults(subcommand_handler=create_project_handler)
+ create_project_parser.add_argument(
+ "PROJECT_DIR",
+ help="Project dir where the new project based on the template dir will be created.",
+ )
+ create_project_parser.add_argument("MLF", help="MLF .tar archive.")
+ create_project_parser.add_argument(
+ "-f",
+ "--force",
+ action="store_true",
+ help="force project creating even if the specified PROJECT_DIR already exists.",
+ )
+
+ # 'build' subcommand
+ build_parser = micro_parser.add_parser(
+ "build",
+ help="build a project dir, generally creating an image to be flashed, e.g. zephyr.elf.",
+ )
+ build_parser.set_defaults(subcommand_handler=build_handler)
+ build_parser.add_argument("PROJECT_DIR", help="Project dir to build.")
+ build_parser.add_argument("-f", "--force", action="store_true", help="Force rebuild.")
+
+ # 'flash' subcommand
+ flash_parser = micro_parser.add_parser(
+ "flash", help="flash the built image on a given micro target."
+ )
+ flash_parser.set_defaults(subcommand_handler=flash_handler)
+ flash_parser.add_argument("PROJECT_DIR", help="Project dir with a image built.")
+
+ # TODO(gromero): list and select serial when multiple devices exist
+ # It's not clear yet if the device list should be determined by TVMC or returned by the Project API
+
+ # For each platform add arguments detected automatically using Project API info query.
+
+ # Create subparsers for the platforms under 'create-project', 'build', and 'flash' subcommands first.
+ help_msg = "you must selected a platform from the list. You can pass '-h' for a selected platform to list its options."
+ create_project_platforms_parser = create_project_parser.add_subparsers(
+ title="platforms", help=help_msg, dest="platform"
+ )
+ build_platforms_parser = build_parser.add_subparsers(
+ title="platforms", help=help_msg, dest="platform"
+ )
+ flash_platforms_parser = flash_parser.add_subparsers(
+ title="platforms", help=help_msg, dest="platform"
+ )
+
+ # Map used to map a API method to proper CLI parsers and handlers.
+ # API method parser handler smbcmd domain
+ subcmds = {
+ "generate_project": [create_project_platforms_parser, create_project_handler],
+ "build": [build_platforms_parser, build_handler],
+ "flash": [flash_platforms_parser, flash_handler],
+ }
+
+ # Helper to add a platform to a subcmd parser.
+ def _add_parser(parser, platform):
+ platform_name = platform[0].upper() + platform[1:] + " platform"
+ o = parser.add_parser(platform, add_help=False, help=f"select {platform_name}.")
+ o.set_defaults(platform=platform)
+ return o
+
+ parser_by_subcmd = {}
+ for subcmd, o in subcmds.items():
+ parser_by_platform = {}
+ o[0].required = True # Selecting a platform/template is mandatory
+ for platform in TEMPLATES:
+ new_parser = _add_parser(o[0], platform)
+ parser_by_platform[platform] = new_parser
+
+ new_parser = o[0].add_parser("template", add_help=False, help="select an adhoc template.")
+ new_parser.add_argument(
+ "-d", metavar="TEMPLATE_DIR", required=True, help="project API template directory."
Review comment:
Yep. Done in https://github.com/apache/tvm/pull/9229/commits/e2acda088729eaf540c9dd87608ab36165eec75c
--
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] mehrdadh commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735925540
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,278 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+from .fmtopt import format_option
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
+ARDUINO_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/arduino/template_project"
+
+
+TEMPLATES = {
+ "zephyr": ZEPHYR_TEMPLATE_DIR,
+ "arduino": ARDUINO_TEMPLATE_DIR,
+}
+
+
+@register_parser
+def add_micro_parser(subparsers, main_parser):
+ micro = subparsers.add_parser("micro", help="select micro context.")
+ micro.set_defaults(func=drive_micro)
+
+ micro_parser = micro.add_subparsers(title="subcommands")
+ # Selecting a subcommand under 'micro' is mandatory
+ micro_parser.required = True
+ micro_parser.dest = "subcommand" # options available to select
+
+ # 'create_project' subcommand
+ create_project_parser = micro_parser.add_parser(
+ "create-project", help="create a project template of a given type or given a template dir."
+ )
+ create_project_parser.set_defaults(subcommand_handler=create_project_handler)
+ create_project_parser.add_argument(
+ "PROJECT_DIR",
+ help="Project dir where the new project based on the template dir will be created.",
+ )
+ create_project_parser.add_argument("MLF", help="MLF .tar archive.")
+ create_project_parser.add_argument(
+ "-f",
+ "--force",
+ action="store_true",
+ help="force project creating even if the specified PROJECT_DIR already exists.",
+ )
+
+ # 'build' subcommand
+ build_parser = micro_parser.add_parser(
+ "build",
+ help="build a project dir, generally creating an image to be flashed, e.g. zephyr.elf.",
+ )
+ build_parser.set_defaults(subcommand_handler=build_handler)
+ build_parser.add_argument("PROJECT_DIR", help="Project dir to build.")
+ build_parser.add_argument("-f", "--force", action="store_true", help="Force rebuild.")
+
+ # 'flash' subcommand
+ flash_parser = micro_parser.add_parser(
+ "flash", help="flash the built image on a given micro target."
+ )
+ flash_parser.set_defaults(subcommand_handler=flash_handler)
+ flash_parser.add_argument("PROJECT_DIR", help="Project dir with a image built.")
+
+ # TODO(gromero): list and select serial when multiple devices exist
+ # It's not clear yet if the device list should be determined by TVMC or returned by the Project API
Review comment:
@gromero you're right. It should be in project API and added automatically to tvmc.
--
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] mehrdadh commented on pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#issuecomment-954300480
Please add `[microtvm]` to PR title.
--
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] gromero commented on pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#issuecomment-961512197
@areusch I've removed the ugly "lookahead" hack and replaced it by a real parser in https://github.com/apache/tvm/pull/9229/commits/409ee6c768517df37300cb6fa529f716e31e7751
The change also now allows using `-h` or `--help` to list the platform options like:
```
gromero@amd:~/git/tvm$ tvmc micro create-project --force /tmp/x46 ~/scripts/sine.tar zephyr --help
usage: tvmc micro create-project project_dir MLF zephyr --project-option OPTION=VALUE [OPTION=VALUE ...] [-h]
optional arguments:
--project-option OPTION=VALUE [OPTION=VALUE ...]
extra_files_tar=EXTRA_FILES_TAR
if given, during generate_project, uncompress the tarball at this path into the project dir.
project_type={aot_demo, host_driven}
type of project to generate. (required)
west_cmd=WEST_CMD
path to the west tool. If given, supersedes both the zephyr_base option and ZEPHYR_BASE environment variable. Defaults to '/usr/bin/python3 -m west'.
zephyr_board={mimxrt1050_evk, mps2_an521, nrf5340dk_nrf5340_cpuapp,
nucleo_f746zg, nucleo_l4r5zi, qemu_cortex_r5, qemu_riscv32,
qemu_riscv64, qemu_x86, stm32f746g_disco}
name of the Zephyr board to build for. (required)
config_main_stack_size=CONFIG_MAIN_STACK_SIZE
sets CONFIG_MAIN_STACK_SIZE for Zephyr board.
-h, --help, --list-options
show this help message with platform-specific options and exit.
```
Which was done in https://github.com/apache/tvm/pull/9229/commits/99f701fd7efc8be34e1e142210ae07f360427898
--
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] areusch commented on a change in pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r748590164
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -109,21 +168,58 @@ def drive_run(args):
Arguments from command line parser.
"""
- rpc_hostname, rpc_port = common.tracker_host_port_from_cli(args.rpc_tracker)
+ path = pathlib.Path(args.PATH)
+ options = None
+ if args.device == "micro":
+ path = path / "model.tar"
Review comment:
we can fix this in a follow-on, but i think this should be read from ServerInfo.model_library_format_path. sorry for missing that before.
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -366,79 +464,102 @@ def run_module(
"Try calling tvmc.compile on the model before running it."
)
- # Currently only two package formats are supported: "classic" and
- # "mlf". The later can only be used for micro targets, i.e. with microTVM.
- if tvmc_package.type == "mlf":
Review comment:
can also do this in a follow-on. i think you still need to preserve this check when device != "micro"
--
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] gromero commented on pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#issuecomment-961512197
@areusch I've removed the ugly "lookahead" hack and replaced it by a real parser in https://github.com/apache/tvm/pull/9229/commits/409ee6c768517df37300cb6fa529f716e31e7751
The change also now allows using `-h` or `--help` to list the platform options like:
```
gromero@amd:~/git/tvm$ tvmc micro create-project --force /tmp/x46 ~/scripts/sine.tar zephyr --help
usage: tvmc micro create-project project_dir MLF zephyr --project-option OPTION=VALUE [OPTION=VALUE ...] [-h]
optional arguments:
--project-option OPTION=VALUE [OPTION=VALUE ...]
extra_files_tar=EXTRA_FILES_TAR
if given, during generate_project, uncompress the tarball at this path into the project dir.
project_type={aot_demo, host_driven}
type of project to generate. (required)
west_cmd=WEST_CMD
path to the west tool. If given, supersedes both the zephyr_base option and ZEPHYR_BASE environment variable. Defaults to '/usr/bin/python3 -m west'.
zephyr_board={mimxrt1050_evk, mps2_an521, nrf5340dk_nrf5340_cpuapp,
nucleo_f746zg, nucleo_l4r5zi, qemu_cortex_r5, qemu_riscv32,
qemu_riscv64, qemu_x86, stm32f746g_disco}
name of the Zephyr board to build for. (required)
config_main_stack_size=CONFIG_MAIN_STACK_SIZE
sets CONFIG_MAIN_STACK_SIZE for Zephyr board.
-h, --help, --list-options
show this help message with platform-specific options and exit.
```
Which was done in https://github.com/apache/tvm/pull/9229/commits/99f701fd7efc8be34e1e142210ae07f360427898
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735760958
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -97,7 +111,55 @@ def add_run_parser(subparsers):
help="hostname (required) and port (optional, defaults to 9090) of the RPC tracker, "
"e.g. '192.168.0.100:9999'",
)
- parser.add_argument("FILE", help="path to the compiled module file")
+ parser.add_argument(
+ "PATH",
+ help="path to the compiled module file or to the project directory (micro devices only).",
+ )
+
+ #
+ # Hack to lookahead if 'run' was selected and make
+ # two dynamic parsers (in micro.py and in runner.py) work.
+ #
+ # print(sys.argv)
+ if "run" in sys.argv:
Review comment:
That is a tricky one. The tricky thing here is that due to tvmc design - that uses a decorator to build a list of functions that will later be called explicitly to augment the parser - a call to `parser_known_args()` can not exit when the command line doesn't match, i.e. when a command line doesn't even satisfies the minimum required syntax so `parser_known_args()` can not return, so it exists without even providing a known/unknown list of options.
If `parser_known_args()` returns to early there won't be a chance of calling the second function responsible to build the second dynamic parser (in this case parser in micro.py will be called secondly, as per import order in `__init__.py`). So when the first call to `parser_known_args()` returns and prints the options missing it will miss the options that would be included by the second function responsible to build the second dynamic parser.
The case you've asked about ("what if `run` is e.g. the value of --template-dir?") nicely works:
```
gromero@amd:~/git/tvm$ tvmc micro create-project /tmp/x16 ~/scripts/sine.tar template --template-dir ./apps/microtvm/run_adhoc_template/template_project --project-option zephyr_board=stm32f746g_disco project_type=host_driven
gromero@amd:~/git/tvm$ ls -l /tmp/x16
total 108
-rw-rw-r-- 1 gromero gromero 1404 Oct 25 14:16 boards.json
-rw-rw-r-- 1 gromero gromero 2245 Oct 25 14:30 CMakeLists.txt
drwxrwxr-x 4 gromero gromero 4096 Oct 25 14:30 crt
drwxrwxr-x 2 gromero gromero 4096 Oct 25 14:30 crt_config
-rw-rw-r-- 1 gromero gromero 25434 Oct 25 14:16 microtvm_api_server.py
drwx------ 6 gromero gromero 4096 Jul 27 20:07 model
-rw-rw-r-- 1 gromero gromero 51200 Jul 27 20:07 model.tar
-rw-rw-r-- 1 gromero gromero 408 Oct 25 14:30 prj.conf
drwxrwxr-x 2 gromero gromero 4096 Oct 25 14:16 src
gromero@amd:~/git/tvm$
```
But the corner case is actually something like:
```
gromero@amd:~/git/tvm$ tvmc micro create-project /tmp/x17 ~/scripts/sine.tar template --template-dir ./apps/microtvm/run_adhoc_templatez/template_project -x run
usage: tvmc [-h] [-v] [--version] {tune,compile,run} ...
tvmc: error: invalid choice: 'micro' (choose from 'tune', 'compile', 'run')
```
Note that 'micro' context is not understood because `parse_known_args()` returns before parser in `micro.py` has the chance to augment the parser and add the micro context. If I add the dynamic parser in `micro.py` the dynamic parser `runner.py` will suffer that "vanishment" instead.
However to me the corner case seems to be a very rare to happen, hence the "lookahead" hack.
--
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] mehrdadh commented on pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#issuecomment-951478241
@gromero I wrote a TVMC test which supports both Zephyr and Arduino when I was testing this PR last week:
[TVMC Test](https://github.com/mehrdadh/tvm/commit/66a8bcebd6022cbf96120412d9ab58a56fbbcb42)
Feel free to reuse 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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735738703
##########
File path: python/tvm/driver/tvmc/fmtopt.py
##########
@@ -0,0 +1,70 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from textwrap import TextWrapper
+
+
+def format_option(option_text, help_text, required=True):
+ # Maximum column length starting at the option name.
+ COL_LEN = 80
+
+ # Extract option name (option name + '=' char, i.e. '=' is part of what
+ # is considered here 'optname') plus its length for choices and help
+ # text justification.
+ optname_break = option_text.find("=")
+ optname = option_text[: optname_break + 1] # '=' is part of the option name in this context
+ optname_len = len(optname)
+
+ # Prepare optname + choices text chunck.
+ choices_text = option_text[optname_break + 1 :] # '=' is not part of the choices
+ wrapper = TextWrapper(width=COL_LEN - optname_len)
+ choices_lines = wrapper.wrap(choices_text)
+
+ # Set first choices line which merely appends to optname string.
+ # No justification is necessary for the first line since first
+ # line was wrapped based on COL_LEN - optname_len, i.e. considering
+ # optname_len.
+ choices_just_lines = [optname + choices_lines[0]]
+
+ # Justify the remaining lines based on first optname + '='.
+ for line in choices_lines[1:]:
+ line_len = len(line)
+ line_just = line.rjust(
+ optname_len + 1 + line_len
+ ) # add 1 to align after '{' in the line above
+ choices_just_lines.append(line_just)
+
+ choices_text_just_chunk = "\n".join(choices_just_lines)
+
+ # Prepare help text chunck.
+ wrapper = TextWrapper(width=120)
Review comment:
Initial idea was to wrap also the help text line (not just option choices) and justify it so it would be below the optname/chnoices text. But later I realized that it would look odd when it was displayed with other non-format help text directly added by `argparse`. So I picked up 120 cause it was an adhoc value that would cause no line wrapping...
I've refactor a bit the code so now there is a separate module parameter `MAX_HELP_TEXT_COL_LEN` (https://github.com/apache/tvm/pull/9229/commits/287b643c4c1f37620f32fae1fa242af5e99df057#diff-829b6ec83c95abdda9185a08f28756b93c96e4a5b2b66ce502ee3cb43e8918b7R28) that can be used to control the help text line wrapping and it's currently set to `0`, which means it's disabled.
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735733194
##########
File path: python/tvm/driver/tvmc/fmtopt.py
##########
@@ -0,0 +1,70 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from textwrap import TextWrapper
+
+
+def format_option(option_text, help_text, required=True):
+ # Maximum column length starting at the option name.
+ COL_LEN = 80
+
+ # Extract option name (option name + '=' char, i.e. '=' is part of what
+ # is considered here 'optname') plus its length for choices and help
+ # text justification.
+ optname_break = option_text.find("=")
Review comment:
Thanks, as you suggested is much better.
Done in also in https://github.com/apache/tvm/pull/9229/commits/287b643c4c1f37620f32fae1fa242af5e99df057#diff-829b6ec83c95abdda9185a08f28756b93c96e4a5b2b66ce502ee3cb43e8918b7R37
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735729288
##########
File path: apps/microtvm/zephyr/template_project/microtvm_api_server.py
##########
@@ -229,40 +229,69 @@ def _get_nrf_device_args(options):
PROJECT_OPTIONS = [
server.ProjectOption(
"extra_files_tar",
+ optional=["generate_project"],
+ type="str",
help="If given, during generate_project, uncompress the tarball at this path into the project dir.",
),
server.ProjectOption(
- "gdbserver_port", help=("If given, port number to use when running the local gdbserver.")
+ "gdbserver_port",
+ help=("If given, port number to use when running the local gdbserver."),
+ optional=["open_transport"],
+ type="int",
),
server.ProjectOption(
"nrfjprog_snr",
+ optional=["open_transport"],
+ type="int",
help=("When used with nRF targets, serial # of the attached board to use, from nrfjprog."),
),
server.ProjectOption(
"openocd_serial",
+ optional=["open_transport"],
+ type="int",
help=("When used with OpenOCD targets, serial # of the attached board to use."),
),
server.ProjectOption(
"project_type",
- help="Type of project to generate.",
choices=tuple(PROJECT_TYPES),
+ required=["generate_project"],
+ type="str",
+ help="Type of project to generate.",
+ ),
+ server.ProjectOption(
+ "verbose",
+ optional=["build"],
+ help="Run build with verbose output.",
+ type="bool",
),
- server.ProjectOption("verbose", help="Run build with verbose output.", choices=(True, False)),
server.ProjectOption(
"west_cmd",
+ optional=["generate_project"],
+ default="python3 -m west",
+ type="str",
help=(
"Path to the west tool. If given, supersedes both the zephyr_base "
"option and ZEPHYR_BASE environment variable."
),
),
- server.ProjectOption("zephyr_base", help="Path to the zephyr base directory."),
+ server.ProjectOption(
+ "zephyr_base",
+ optional=["build", "open_transport"],
+ default="ZEPHYR_BASE",
Review comment:
Hence now it shows:
```
zephyr_base=ZEPHYR_BASE
path to the zephyr base directory. Defaults to '/home/gromero/zephyrproject/zephyr'.
```
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r738593504
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -384,13 +482,27 @@ def run_module(
else:
logger.debug("Running on remote RPC with no key.")
session = rpc.connect(hostname, port)
+ elif device == "micro":
+ # Remote RPC (running on a micro target)
+ logger.debug("Running on remote RPC (micro target).")
+ try:
+ with ExitStack() as stack:
Review comment:
oh got it. thanks, I misunderstood the purpose of `contextlib` the first time.
So, I've put all below line 475 inside the `with` scope as you suggested. Just a note that was precisely something like that I was trying to avoid since the beginning, that's why I thought of introducing `open()` and `close()` methods at first then use them explicitly in `runner.py`, first when opening the transport and creating a session, than later at end of `run_module`, checking again if `device == "micro" and if so calling `close()` explicitly. But yeah If you think that putting everything after line 475 inside `with` is ok, alright then.
--
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] gromero commented on a change in pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r749833223
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -366,79 +464,102 @@ def run_module(
"Try calling tvmc.compile on the model before running it."
)
- # Currently only two package formats are supported: "classic" and
- # "mlf". The later can only be used for micro targets, i.e. with microTVM.
- if tvmc_package.type == "mlf":
Review comment:
yeah, that seems correct. Ok, I'll prepare a follow-on PR for it. :+1:
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735775515
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -404,16 +513,24 @@ def run_module(
dev = session.vulkan()
elif device == "rocm":
dev = session.rocm()
+ elif device == "micro":
+ dev = session.device
+ lib = session.get_system_lib()
else:
assert device == "cpu"
dev = session.cpu()
+ # TODO(gromero): Adjust for micro targets.
Review comment:
For now. I've reverted to use the common code (and so use `module.benchmark()`): https://github.com/apache/tvm/pull/9229/commits/825376f680cb8f174bf3e5b6c6ef707f78c86cc6
And kept the workaround until we decide what to do so the branch is kept working: https://github.com/apache/tvm/pull/9229/commits/a63a48639bd8d5eda351d443c7ea4f8f2bfdfdbf
--
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] gromero closed pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero closed pull request #9229:
URL: https://github.com/apache/tvm/pull/9229
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r738586511
##########
File path: python/tvm/micro/project.py
##########
@@ -75,14 +75,23 @@ def __init__(self, api_client, options):
raise TemplateProjectError()
def build(self):
+ assert self._options is not None, "'options' is not set!"
self._api_client.build(self._options)
def flash(self):
+ assert self._options is not None, "'options' is not set!"
self._api_client.flash(self._options)
def transport(self):
+ assert self._options is not None, "'options' is not set!"
return ProjectTransport(self._api_client, self._options)
+ def info(self):
+ return self._info
+
+ def set_options(self, options):
Review comment:
Sure. 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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] gromero commented on a change in pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r752631424
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -366,79 +464,102 @@ def run_module(
"Try calling tvmc.compile on the model before running it."
)
- # Currently only two package formats are supported: "classic" and
- # "mlf". The later can only be used for micro targets, i.e. with microTVM.
- if tvmc_package.type == "mlf":
Review comment:
@areusch Actually, it's fixed in this PR, otherwise tests fail ;)
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735790881
##########
File path: src/runtime/rpc/rpc_module.cc
##########
@@ -164,7 +164,7 @@ class RPCModuleNode final : public ModuleNode {
~RPCModuleNode() {
if (module_handle_ != nullptr) {
try {
- sess_->FreeHandle(module_handle_, kTVMModuleHandle);
+ // sess_->FreeHandle(module_handle_, kTVMModuleHandle);
Review comment:
This isn't actually a nit :(
Uncommenting it amounts to revert https://github.com/apache/tvm/pull/9229/commits/ce29d5b8be2941f68f2587d5ec218e988dcb11b6
This obviously can't get merged and needs a proper fix. But I don't have a suggestion yet. As we discussed sometime ago we could flash every time before we run a model but that seems overkill to me.
Although we've chatted a bit about potential memory leaks when running over and over a model without resetting the device, on my experiments I never have an issue when running multiple times with that workaround, (I let the models run overnight, a hundred times in sequence, without resetting the device or flashing the model again between the runs). But yeah I do agree a deeper investigation is necessary here.
Another option I thought of is to introduce a new packet to reset the device, but I'm not sure when exactly it would be send to the device (if in the `RPCModuleNode` constructor or somewhere else).
Suggestions welcome :)
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735790881
##########
File path: src/runtime/rpc/rpc_module.cc
##########
@@ -164,7 +164,7 @@ class RPCModuleNode final : public ModuleNode {
~RPCModuleNode() {
if (module_handle_ != nullptr) {
try {
- sess_->FreeHandle(module_handle_, kTVMModuleHandle);
+ // sess_->FreeHandle(module_handle_, kTVMModuleHandle);
Review comment:
This isn't actually a nit :(
Uncommenting it amounts to reverting https://github.com/apache/tvm/pull/9229/commits/ce29d5b8be2941f68f2587d5ec218e988dcb11b6
This obviously can't get merged and needs a proper fix. But I don't have a suggestion yet. As we discussed sometime ago we could flash every time before we run a model but that seems overkill to me.
Although we've chatted a bit about potential memory leaks when running over and over a model without resetting the device, on my experiments I never have an issue when running multiple times with that workaround, (I let the models run overnight, a hundred times in sequence, without resetting the device or flashing the model again between the runs). But yeah I do agree a deeper investigation is necessary here.
Another option I thought of is to introduce a new packet to reset the device, but I'm not sure when exactly it would be send to the device (if in the `RPCModuleNode` constructor or somewhere else).
Suggestions welcome :)
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735807426
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -404,16 +513,24 @@ def run_module(
dev = session.vulkan()
elif device == "rocm":
dev = session.rocm()
+ elif device == "micro":
+ dev = session.device
+ lib = session.get_system_lib()
else:
assert device == "cpu"
dev = session.cpu()
+ # TODO(gromero): Adjust for micro targets.
Review comment:
@areusch Do you have any suggestion on how to fix 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] gromero commented on pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#issuecomment-951136485
> overall looks great. I did a partial pass, will do another later this week. Just wanted to provide some early feedbacks.
>
> When I run `-h` after the platform(zephyr,arduino,temolate) I do see error message instead of showing the help message:
>
> ```
> - (microtvm-gTppbK-Q-py3.6) vagrant@microtvm:/home/mhessar/tvm$ tvmc micro create-project output_directory micro_speech.tar zephyr -h
> usage: tvmc micro create-project PROJECT_DIR MLF zephyr [--list-options] -o
> OPTION=VALUE
> [OPTION=VALUE ...]
> tvmc micro create-project PROJECT_DIR MLF zephyr: error: the following arguments are required: -o
>
>
>
> - (microtvm-gTppbK-Q-py3.6) vagrant@microtvm:/home/mhessar/tvm$ tvmc micro build /tmp/mehrdad zephyr -h
> usage: tvmc micro build PROJECT_DIR zephyr [--list-options] -o OPTION=VALUE
> [OPTION=VALUE ...]
> tvmc micro build PROJECT_DIR zephyr: error: the following arguments are required: -o
>
>
> - (microtvm-gTppbK-Q-py3.6) vagrant@microtvm:/home/mhessar/tvm$ tvmc micro flash /tmp/mehrdad zephyr -h
> usage: tvmc micro flash PROJECT_DIR zephyr [--list-options] -o OPTION=VALUE
> [OPTION=VALUE ...]
> tvmc micro flash PROJECT_DIR zephyr: error: the following arguments are required: -o
> ```
>
> My expectation was to see the options and requirements just like before adding platform which was like this:
>
> ```
> (microtvm-gTppbK-Q-py3.6) vagrant@microtvm:/home/mhessar/tvm$ tvmc micro build /tmp/mehrdad -h
> usage: tvmc micro build [-h] [-f] PROJECT_DIR {zephyr,arduino,template} ...
>
> positional arguments:
> PROJECT_DIR Project dir to build.
>
> optional arguments:
> -h, --help show this help message and exit
> -f, --force Force rebuild.
>
> platforms:
> {zephyr,arduino,template}
> you must selected a platform from the list. You can
> pass '-h' for a selected platform to list its options.
> zephyr select Zephyr platform.
> arduino select Arduino platform.
> template select an adhoc template.
> ```
>
> please let me know what you think.
This is the same behavior we currently have for other `tvmc` commands, like `compile` and `run`. Just when only `tvmc` is typed a complete help message is displayed:
```
gromero@amd:~/git/tvm$ tvmc run
[17:17:15] /home/gromero/git/tvm/src/target/target_kind.cc:164: Warning: Unable to detect CUDA version, default to "-mcpu=sm_20" instead
[17:17:15] /home/gromero/git/tvm/src/target/target_kind.cc:190: Warning: Unable to detect ROCm compute arch, default to "-mcpu=gfx900" instead
[17:17:15] /home/gromero/git/tvm/src/target/target_kind.cc:204: Warning: Unable to detect ROCm version, assuming >= 3.5
[17:17:15] /home/gromero/git/tvm/src/target/target_kind.cc:164: Warning: Unable to detect CUDA version, default to "-mcpu=sm_20" instead
[17:17:15] /home/gromero/git/tvm/src/target/target_kind.cc:190: Warning: Unable to detect ROCm compute arch, default to "-mcpu=gfx900" instead
[17:17:15] /home/gromero/git/tvm/src/target/target_kind.cc:204: Warning: Unable to detect ROCm version, assuming >= 3.5
usage: tvmc run [-h] [--device {cpu,cuda,cl,metal,vulkan,rocm}] [--fill-mode {zeros,ones,random}] [-i INPUTS] [-o OUTPUTS] [--print-time] [--print-top N] [--profile] [--repeat N] [--number N]
[--rpc-key RPC_KEY] [--rpc-tracker RPC_TRACKER]
FILE
tvmc run: error: the following arguments are required: FILE
gromero@amd:~/git/tvm$ tvmc
[17:17:20] /home/gromero/git/tvm/src/target/target_kind.cc:164: Warning: Unable to detect CUDA version, default to "-mcpu=sm_20" instead
[17:17:20] /home/gromero/git/tvm/src/target/target_kind.cc:190: Warning: Unable to detect ROCm compute arch, default to "-mcpu=gfx900" instead
[17:17:20] /home/gromero/git/tvm/src/target/target_kind.cc:204: Warning: Unable to detect ROCm version, assuming >= 3.5
[17:17:20] /home/gromero/git/tvm/src/target/target_kind.cc:164: Warning: Unable to detect CUDA version, default to "-mcpu=sm_20" instead
[17:17:20] /home/gromero/git/tvm/src/target/target_kind.cc:190: Warning: Unable to detect ROCm compute arch, default to "-mcpu=gfx900" instead
[17:17:20] /home/gromero/git/tvm/src/target/target_kind.cc:204: Warning: Unable to detect ROCm version, assuming >= 3.5
usage: tvmc [-h] [-v] [--version] {tune,compile,run} ...
TVM compiler driver
optional arguments:
-h, --help show this help message and exit
-v, --verbose increase verbosity
--version print the version and exit
commands:
{tune,compile,run}
tune auto-tune a model
compile compile a model.
run run a compiled module
TVMC - TVM driver command-line interface
gromero@amd:~/git/tvm$
```
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735772766
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -404,16 +513,24 @@ def run_module(
dev = session.vulkan()
elif device == "rocm":
dev = session.rocm()
+ elif device == "micro":
+ dev = session.device
+ lib = session.get_system_lib()
else:
assert device == "cpu"
dev = session.cpu()
+ # TODO(gromero): Adjust for micro targets.
Review comment:
Yeah. I meant here that currently `module.benchmark(dev, number=number, repeat=repeat)` as used by the other targets is not working for micro targets and will throw the following trace:
```
Traceback (most recent call last):
File "/home/gromero/scripts/./test.py", line 50, in <module>
times = module.benchmark(dev, number=1, repeat=1)
File "/home/gromero/git/tvm/python/tvm/contrib/graph_executor.py", line 407, in benchmark
return self.module.time_evaluator(
File "/home/gromero/git/tvm/python/tvm/runtime/module.py", line 292, in evaluator
blob = feval(*args)
File "tvm/_ffi/_cython/./packed_func.pxi", line 323, in tvm._ffi._cy3.core.PackedFuncBase.__call__
File "tvm/_ffi/_cython/./packed_func.pxi", line 257, in tvm._ffi._cy3.core.FuncCall
File "tvm/_ffi/_cython/./packed_func.pxi", line 246, in tvm._ffi._cy3.core.FuncCall3
File "tvm/_ffi/_cython/./base.pxi", line 163, in tvm._ffi._cy3.core.CALL
tvm._ffi.base.TVMError: Traceback (most recent call last):
3: TVMFuncCall
2: _ZNSt17_Function_handlerIFvN3tvm
1: tvm::runtime::WrapTimeEvaluator(tvm::runtime::PackedFunc, DLDevice, int, int, int, tvm::runtime::PackedFunc)::{lambda(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*)#1}::operator()(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*)
0: tvm::runtime::Timer::Start(DLDevice)
File "/home/gromero/git/tvm/include/tvm/runtime/device_api.h", line 272
TVMError: unknown type =129
```
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735774162
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -404,16 +513,24 @@ def run_module(
dev = session.vulkan()
elif device == "rocm":
dev = session.rocm()
+ elif device == "micro":
+ dev = session.device
+ lib = session.get_system_lib()
else:
assert device == "cpu"
dev = session.cpu()
+ # TODO(gromero): Adjust for micro targets.
Review comment:
Merely adding the DLDevice type `129` will workaround the issue but it feels really wrong. Micro target are defined as simple "CPU" device types, so I could not find from where exactly `129` is being set.
--
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] gromero commented on pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#issuecomment-954735900
> Please add `[microtvm]` to PR title.
I think it's ok to have two tags (
--
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] mehrdadh commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r734753773
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -366,13 +460,17 @@ def run_module(
"Try calling tvmc.compile on the model before running it."
)
- # Currently only two package formats are supported: "classic" and
- # "mlf". The later can only be used for micro targets, i.e. with microTVM.
- if tvmc_package.type == "mlf":
- raise TVMCException(
- "You're trying to run a model saved using the Model Library Format (MLF)."
- "MLF can only be used to run micro targets (microTVM)."
- )
+ micro = False
Review comment:
maybe reuse `device` and get rid of `micro` variable?
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,277 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
Review comment:
you can update these once https://github.com/apache/tvm/pull/9309 is merged.
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735774162
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -404,16 +513,24 @@ def run_module(
dev = session.vulkan()
elif device == "rocm":
dev = session.rocm()
+ elif device == "micro":
+ dev = session.device
+ lib = session.get_system_lib()
else:
assert device == "cpu"
dev = session.cpu()
+ # TODO(gromero): Adjust for micro targets.
Review comment:
Merely adding the DLDevice type `129` will work around the issue but it feels really wrong. Micro target are defined as simple "CPU" device types, so I could not find from where exactly `129` is being set.
--
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] mehrdadh commented on a change in pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r740393544
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,277 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
Review comment:
https://github.com/apache/tvm/pull/9309 is merged 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] gromero commented on a change in pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r746197628
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -38,6 +41,36 @@
class TVMCException(Exception):
"""TVMC Exception"""
+class TVMCSilentArgumentParser(argparse.ArgumentParser):
Review comment:
yep, I've tried it once, didn't work well because I would have to tweak, as you said, _every_ TVMC existing parser/subparser so I gave up on this route. But yeah I'm aware that `_remove_action` is private and so it has issues. So let me give a second try on it. Thanks for the suggestion!
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735760958
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -97,7 +111,55 @@ def add_run_parser(subparsers):
help="hostname (required) and port (optional, defaults to 9090) of the RPC tracker, "
"e.g. '192.168.0.100:9999'",
)
- parser.add_argument("FILE", help="path to the compiled module file")
+ parser.add_argument(
+ "PATH",
+ help="path to the compiled module file or to the project directory (micro devices only).",
+ )
+
+ #
+ # Hack to lookahead if 'run' was selected and make
+ # two dynamic parsers (in micro.py and in runner.py) work.
+ #
+ # print(sys.argv)
+ if "run" in sys.argv:
Review comment:
That is a tricky one. The tricky thing here is that due to tvmc design - that uses a decorator to build a list of functions that will later be called explicitly to augment the parser - a call to `parser_known_args()` can not exit when the command line doesn't match, i.e. when a command line doesn't even satisfies the minimum required syntax so `parser_known_args()` can return, so it exists and can't even provide a known/unknown list of options.
If `parser_known_args()` returns to early there won't be a chance of calling the second function responsible to build the second dynamic parser (in this case parser in micro.py will be called secondly, as per import order in `__init__.py`). So when the first call to `parser_known_args()` returns and prints the options missing it will miss the options that would be included by the second function responsible to build the second dynamic parser.
The case you've asked about ("what if `run` is e.g. the value of --template-dir?") nicely works:
``gromero@amd:~/git/tvm$ tvmc micro create-project /tmp/x16 ~/scripts/sine.tar template --template-dir ./apps/microtvm/run_adhoc_template/template_project --project-option zephyr_board=stm32f746g_disco project_type=host_driven
gromero@amd:~/git/tvm$ ls -l /tmp/x16
total 108
-rw-rw-r-- 1 gromero gromero 1404 Oct 25 14:16 boards.json
-rw-rw-r-- 1 gromero gromero 2245 Oct 25 14:30 CMakeLists.txt
drwxrwxr-x 4 gromero gromero 4096 Oct 25 14:30 crt
drwxrwxr-x 2 gromero gromero 4096 Oct 25 14:30 crt_config
-rw-rw-r-- 1 gromero gromero 25434 Oct 25 14:16 microtvm_api_server.py
drwx------ 6 gromero gromero 4096 Jul 27 20:07 model
-rw-rw-r-- 1 gromero gromero 51200 Jul 27 20:07 model.tar
-rw-rw-r-- 1 gromero gromero 408 Oct 25 14:30 prj.conf
drwxrwxr-x 2 gromero gromero 4096 Oct 25 14:16 src
gromero@amd:~/git/tvm$
```
But the corner case is actually something like:
```
gromero@amd:~/git/tvm$ tvmc micro create-project /tmp/x17 ~/scripts/sine.tar template --template-dir ./apps/microtvm/run_adhoc_templatez/template_project -x run
usage: tvmc [-h] [-v] [--version] {tune,compile,run} ...
tvmc: error: invalid choice: 'micro' (choose from 'tune', 'compile', 'run')
```
Note that 'micro' context is not understood because `parse_known_args()` returns before parser in `micro.py` has the chance to augment the parser and add the micro context. If I add the dynamic parser in `micro.py` the dynamic parser `runner.py` will suffer that "vanishment" instead.
However to me the corner case seems to be a very rare to happen, hence the "lookahead" hack.
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735732431
##########
File path: python/tvm/driver/tvmc/fmtopt.py
##########
@@ -0,0 +1,70 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from textwrap import TextWrapper
+
+
+def format_option(option_text, help_text, required=True):
+ # Maximum column length starting at the option name.
+ COL_LEN = 80
Review comment:
I've also s/`COL_LEN`/`MAX_OPTNAME_CHOICES_TEXT_COL_LEN`/
Done in https://github.com/apache/tvm/pull/9229/commits/287b643c4c1f37620f32fae1fa242af5e99df057
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735739453
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,278 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+from .fmtopt import format_option
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
+ARDUINO_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/arduino/template_project"
+
+
+TEMPLATES = {
+ "zephyr": ZEPHYR_TEMPLATE_DIR,
+ "arduino": ARDUINO_TEMPLATE_DIR,
+}
+
+
+@register_parser
+def add_micro_parser(subparsers, main_parser):
+ micro = subparsers.add_parser("micro", help="select micro context.")
+ micro.set_defaults(func=drive_micro)
+
+ micro_parser = micro.add_subparsers(title="subcommands")
+ # Selecting a subcommand under 'micro' is mandatory
+ micro_parser.required = True
+ micro_parser.dest = "subcommand" # options available to select
+
+ # 'create_project' subcommand
+ create_project_parser = micro_parser.add_parser(
+ "create-project", help="create a project template of a given type or given a template dir."
+ )
+ create_project_parser.set_defaults(subcommand_handler=create_project_handler)
+ create_project_parser.add_argument(
+ "PROJECT_DIR",
+ help="Project dir where the new project based on the template dir will be created.",
+ )
+ create_project_parser.add_argument("MLF", help="MLF .tar archive.")
Review comment:
Done in https://github.com/apache/tvm/pull/9229/commits/c5fc10aa9906d3f765304f6f3fdfa40b6493e34c
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735720743
##########
File path: apps/microtvm/zephyr/template_project/microtvm_api_server.py
##########
@@ -229,40 +229,69 @@ def _get_nrf_device_args(options):
PROJECT_OPTIONS = [
server.ProjectOption(
"extra_files_tar",
+ optional=["generate_project"],
+ type="str",
Review comment:
The issue here is that str, bool, etc are metaclasses of 'type' type class so they can not themselves be serialized. Hence the following happens if I do the substitution you suggested:
```
gromero@amd:~/git/tvm$ tvmc micro build /tmp/x20 zephyr --list-options
Traceback (most recent call last):
File "/usr/lib/python3.9/runpy.py", line 197, in _run_module_as_main
return _run_code(code, main_globals, None,
File "/usr/lib/python3.9/runpy.py", line 87, in _run_code
exec(code, run_globals)
File "/home/gromero/git/tvm/python/tvm/driver/tvmc/__main__.py", line 24, in <module>
tvmc.main.main()
File "/home/gromero/git/tvm/python/tvm/driver/tvmc/main.py", line 94, in main
sys.exit(_main(sys.argv[1:]))
File "/home/gromero/git/tvm/python/tvm/driver/tvmc/main.py", line 69, in _main
make_subparser(subparser, parser)
File "/home/gromero/git/tvm/python/tvm/driver/tvmc/micro.py", line 173, in add_micro_parser
template = project.TemplateProject.from_directory(template_dir)
File "/home/gromero/git/tvm/python/tvm/micro/project.py", line 105, in from_directory
return cls(client.instantiate_from_dir(template_project_dir))
File "/home/gromero/git/tvm/python/tvm/micro/project.py", line 109, in __init__
self._info = self._api_client.server_info_query(__version__)
File "/home/gromero/git/tvm/python/tvm/micro/project_api/client.py", line 143, in server_info_query
reply = self._request_reply("server_info_query", {"tvm_version": tvm_version})
File "/home/gromero/git/tvm/python/tvm/micro/project_api/client.py", line 135, in _request_reply
raise server.JSONRPCError.from_json(f"calling method {method}", reply["error"])
tvm.micro.project_api.server.ServerError: calling method server_info_query: JSON-RPC error # -32000: calling method server_info_query
Traceback (most recent call last):
File "/home/gromero/git/tvm/python/tvm/micro/project_api/server.py", line 479, in serve_one_request # <--- Outermost server-side stack frame
self._dispatch_request(request)
File "/home/gromero/git/tvm/python/tvm/micro/project_api/server.py", line 592, in _dispatch_request
self._write_reply(request["id"], result=return_value)
File "/home/gromero/git/tvm/python/tvm/micro/project_api/server.py", line 608, in _write_reply
reply_str = json.dumps(reply_dict)
File "/usr/lib/python3.9/json/__init__.py", line 231, in dumps
return _default_encoder.encode(obj)
File "/usr/lib/python3.9/json/encoder.py", line 199, in encode
chunks = self.iterencode(o, _one_shot=True)
File "/usr/lib/python3.9/json/encoder.py", line 257, in iterencode
return _iterencode(o, 0)
File "/usr/lib/python3.9/json/encoder.py", line 179, in default
raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type type is not JSON serializable
```
--
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] gromero commented on pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#issuecomment-951133986
> Added few comments. Please address previous comments. Also, how do we test tvmc on CI?
I plan to add the tests along the lines of the existing tvmc tests, but for subcommands like `flash` and `run`, to really exercise the code a real physical device (microTVM CI) will be required. `create-project` and `build` are easier to test.
I'll do that once we set agreement on the interface and other implementation details etc
--
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] gromero commented on pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#issuecomment-954048032
@areusch about modifying TVMModFree to be a no-op when mod == system_lib_handle: it works. Done in https://github.com/apache/tvm/pull/9229/commits/e76156d2fd872d2263c71a0eadd4f138773e06d6
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r738586393
##########
File path: python/tvm/micro/project_api/server.py
##########
@@ -42,15 +42,24 @@
_LOG = logging.getLogger(__name__)
-_ProjectOption = collections.namedtuple("ProjectOption", ("name", "choices", "help"))
+_ProjectOption = collections.namedtuple(
+ "ProjectOption", ("name", "choices", "default", "type", "required", "optional", "help")
+)
class ProjectOption(_ProjectOption):
+ """Class used to keep the metadata associated to project options."""
+
def __new__(cls, name, **kw):
"""Override __new__ to force all options except name to be specified as kwargs."""
assert "name" not in kw
+ assert "required" in kw or "optional" in kw, "'required' or 'optional' must be specified."
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] gromero commented on a change in pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r746190697
##########
File path: include/tvm/runtime/device_api.h
##########
@@ -268,6 +268,8 @@ inline const char* DeviceName(int type) {
return "webgpu";
case kDLHexagon:
return "hexagon";
+ case 129:
Review comment:
yeah, sorry, I left it while the PR is in Draft state. I'll need to fix it in a follow-on PR tho (I still need to figure out what's exactly missing - fwiw it seems that `module.benchmark()` and so `module.time_evaluator` works fine on micro targets with that change hence it seems not much is missing to make it work the right way). For now I can add `--print-time` to the list of non-supported options for micro targets on `tvmc run`.
--
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] areusch commented on pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#issuecomment-967580455
@leandron in case you want to look at this too
--
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] gromero commented on a change in pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r747901711
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -38,6 +41,36 @@
class TVMCException(Exception):
"""TVMC Exception"""
+class TVMCSilentArgumentParser(argparse.ArgumentParser):
Review comment:
@areusch Hi. PTAL at https://github.com/apache/tvm/pull/9229/commits/ffd19e868adb51505843d8090ec61054c6ef1480
The approach of adding the help options at the last minute works :) Now I think the code is fine. Adding at the last minute for each parser didn't work well, but it was in fact only necessary to do that to the main parser (in main.py) and additionally ignore the `invalid choice` error in the main parser too by overriding the `error` method in `argparse.ArgumentParser`. I've changed the name of `TVMCSilentArgumentParser` to `TVMCSuppressedArgumentParser`, since that class can't suppress all the error, like for instance, the "required argument" error (but that's not necessary).
--
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] areusch merged pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
areusch merged pull request #9229:
URL: https://github.com/apache/tvm/pull/9229
--
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] areusch commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r727495939
##########
File path: apps/microtvm/zephyr/template_project/microtvm_api_server.py
##########
@@ -229,40 +229,69 @@ def _get_nrf_device_args(options):
PROJECT_OPTIONS = [
server.ProjectOption(
"extra_files_tar",
+ optional=["generate_project"],
+ type="str",
help="If given, during generate_project, uncompress the tarball at this path into the project dir.",
),
server.ProjectOption(
- "gdbserver_port", help=("If given, port number to use when running the local gdbserver.")
+ "gdbserver_port",
+ help=("If given, port number to use when running the local gdbserver."),
+ optional=["open_transport"],
+ type="int",
),
server.ProjectOption(
"nrfjprog_snr",
+ optional=["open_transport"],
+ type="int",
help=("When used with nRF targets, serial # of the attached board to use, from nrfjprog."),
),
server.ProjectOption(
"openocd_serial",
+ optional=["open_transport"],
+ type="int",
help=("When used with OpenOCD targets, serial # of the attached board to use."),
),
server.ProjectOption(
"project_type",
- help="Type of project to generate.",
choices=tuple(PROJECT_TYPES),
+ required=["generate_project"],
+ type="str",
+ help="Type of project to generate.",
+ ),
+ server.ProjectOption(
+ "verbose",
+ optional=["build"],
+ help="Run build with verbose output.",
+ type="bool",
),
- server.ProjectOption("verbose", help="Run build with verbose output.", choices=(True, False)),
server.ProjectOption(
"west_cmd",
+ optional=["generate_project"],
+ default="python3 -m west",
Review comment:
is this needed? prefer `sys.executable` if possible
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -520,3 +522,84 @@ def parse_configs(input_configs):
pass_context_configs[name] = parsed_value
return pass_context_configs
+
+
+def get_project_options(project_info):
Review comment:
can you add docstring and type annotation if possible?
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,278 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+from .fmtopt import format_option
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
+ARDUINO_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/arduino/template_project"
+
+
+TEMPLATES = {
+ "zephyr": ZEPHYR_TEMPLATE_DIR,
+ "arduino": ARDUINO_TEMPLATE_DIR,
+}
+
+
+@register_parser
+def add_micro_parser(subparsers, main_parser):
+ micro = subparsers.add_parser("micro", help="select micro context.")
+ micro.set_defaults(func=drive_micro)
+
+ micro_parser = micro.add_subparsers(title="subcommands")
+ # Selecting a subcommand under 'micro' is mandatory
+ micro_parser.required = True
+ micro_parser.dest = "subcommand" # options available to select
+
+ # 'create_project' subcommand
+ create_project_parser = micro_parser.add_parser(
+ "create-project", help="create a project template of a given type or given a template dir."
+ )
+ create_project_parser.set_defaults(subcommand_handler=create_project_handler)
+ create_project_parser.add_argument(
+ "PROJECT_DIR",
+ help="Project dir where the new project based on the template dir will be created.",
+ )
+ create_project_parser.add_argument("MLF", help="MLF .tar archive.")
+ create_project_parser.add_argument(
+ "-f",
+ "--force",
+ action="store_true",
+ help="force project creating even if the specified PROJECT_DIR already exists.",
+ )
+
+ # 'build' subcommand
+ build_parser = micro_parser.add_parser(
+ "build",
+ help="build a project dir, generally creating an image to be flashed, e.g. zephyr.elf.",
+ )
+ build_parser.set_defaults(subcommand_handler=build_handler)
+ build_parser.add_argument("PROJECT_DIR", help="Project dir to build.")
+ build_parser.add_argument("-f", "--force", action="store_true", help="Force rebuild.")
+
+ # 'flash' subcommand
+ flash_parser = micro_parser.add_parser(
+ "flash", help="flash the built image on a given micro target."
+ )
+ flash_parser.set_defaults(subcommand_handler=flash_handler)
+ flash_parser.add_argument("PROJECT_DIR", help="Project dir with a image built.")
+
+ # TODO(gromero): list and select serial when multiple devices exist
+ # It's not clear yet if the device list should be determined by TVMC or returned by the Project API
+
+ # For each platform add arguments detected automatically using Project API info query.
+
+ # Create subparsers for the platforms under 'create-project', 'build', and 'flash' subcommands first.
+ help_msg = "you must selected a platform from the list. You can pass '-h' for a selected platform to list its options."
+ create_project_platforms_parser = create_project_parser.add_subparsers(
+ title="platforms", help=help_msg, dest="platform"
+ )
+ build_platforms_parser = build_parser.add_subparsers(
+ title="platforms", help=help_msg, dest="platform"
+ )
+ flash_platforms_parser = flash_parser.add_subparsers(
+ title="platforms", help=help_msg, dest="platform"
+ )
+
+ # Map used to map a API method to proper CLI parsers and handlers.
+ # API method parser handler smbcmd domain
+ subcmds = {
+ "generate_project": [create_project_platforms_parser, create_project_handler],
+ "build": [build_platforms_parser, build_handler],
+ "flash": [flash_platforms_parser, flash_handler],
+ }
+
+ # Helper to add a platform to a subcmd parser.
+ def _add_parser(parser, platform):
+ platform_name = platform[0].upper() + platform[1:] + " platform"
+ o = parser.add_parser(platform, add_help=False, help=f"select {platform_name}.")
+ o.set_defaults(platform=platform)
+ return o
+
+ parser_by_subcmd = {}
+ for subcmd, o in subcmds.items():
+ parser_by_platform = {}
+ o[0].required = True # Selecting a platform/template is mandatory
+ for platform in TEMPLATES:
+ new_parser = _add_parser(o[0], platform)
+ parser_by_platform[platform] = new_parser
+
+ new_parser = o[0].add_parser("template", add_help=False, help="select an adhoc template.")
+ new_parser.add_argument(
+ "-d", metavar="TEMPLATE_DIR", required=True, help="project API template directory."
+ )
+ new_parser.set_defaults(platform="template")
+ parser_by_platform["template"] = new_parser
+
+ parser_by_subcmd[subcmd] = parser_by_platform
+
+ known_args, unkown_args = main_parser.parse_known_args()
+
+ try:
+ subcmd = known_args.subcommand
+ platform = known_args.platform
+ except AttributeError:
+ # No subcommand or platform, so not need to augment the parser.
+ return
+
+ # Augment parser with project options
+
+ if platform == "template":
+ # adhoc template
+ template_dir = known_args.d
+ else:
+ # default template
+ template_dir = TEMPLATES[platform]
+
+ template = project.TemplateProject.from_directory(template_dir)
+ template_info = template.info()
+
+ options_by_method = get_project_options(template_info)
+
+ # TODO(gromero): refactor to remove this map.
+ subcmd_to_method = {
+ "create-project": "generate_project",
+ "build": "build",
+ "flash": "flash",
+ }
+
+ method = subcmd_to_method[subcmd]
+ parser_by_subcmd_n_platform = parser_by_subcmd[method][platform]
+ _, handler = subcmds[method]
+
+ parser_by_subcmd_n_platform.formatter_class = (
+ argparse.RawTextHelpFormatter
+ ) # Set raw help text so help_text format works
+ parser_by_subcmd_n_platform.set_defaults(
+ subcommand_handler=handler,
+ valid_options=options_by_method[method],
+ template_dir=template_dir,
+ )
+
+ parser_by_subcmd_n_platform.add_argument(
+ "--list-options",
+ action="help",
+ help="show all options/values for selected platforms/template.",
+ )
+
+ required = any([opt["required"] for opt in options_by_method[method]])
+ nargs = "+" if required else "*"
+
+ help_text_by_option = [opt["help_text"] for opt in options_by_method[method]]
+ help_text = "\n\n".join(help_text_by_option) + "\n\n"
+
+ # TODO(gromero): Experiment with required=required below
+ parser_by_subcmd_n_platform.add_argument(
+ "-o", required=True, metavar="OPTION=VALUE", nargs=nargs, help=help_text
Review comment:
i might suggest adding --project-option as an arg name to keep the code more readable
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -520,3 +522,84 @@ def parse_configs(input_configs):
pass_context_configs[name] = parsed_value
return pass_context_configs
+
+
+def get_project_options(project_info):
+ options = project_info["project_options"]
+
+ options_by_method = defaultdict(list)
+ for opt in options:
+ # Get list of methods associated with an option based on the
+ # existance of a 'required' or 'optional' lists. API specification
+ # guarantees at least one of these lists will exist. If a list does
+ # not exist it's returned as None by the API.
+ metadata = ["required", "optional"]
+ om = [(opt[md], True if md == "required" else False) for md in metadata if opt[md]]
+ for methods, is_opt_required in om:
+ for method in methods:
+ name = opt["name"]
+
+ if opt["type"] == "bool":
+ # TODO(gromero): Use only choices= and merge with non-bool options below
+ option_choices_text = f"{name}={{true, false}}"
Review comment:
it might be a bit cleaner to just set `choices = ["true", "false"]` and reuse the logic below.
##########
File path: apps/microtvm/zephyr/template_project/microtvm_api_server.py
##########
@@ -229,40 +229,69 @@ def _get_nrf_device_args(options):
PROJECT_OPTIONS = [
server.ProjectOption(
"extra_files_tar",
+ optional=["generate_project"],
+ type="str",
Review comment:
i think use just `str` not `"str"` here and elsewhere
##########
File path: python/tvm/driver/tvmc/fmtopt.py
##########
@@ -0,0 +1,70 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from textwrap import TextWrapper
+
+
+def format_option(option_text, help_text, required=True):
+ # Maximum column length starting at the option name.
+ COL_LEN = 80
+
+ # Extract option name (option name + '=' char, i.e. '=' is part of what
+ # is considered here 'optname') plus its length for choices and help
+ # text justification.
+ optname_break = option_text.find("=")
Review comment:
cleaner: `optname, chioces_text = option_text.split("=", 1)`
##########
File path: python/tvm/driver/tvmc/fmtopt.py
##########
@@ -0,0 +1,70 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from textwrap import TextWrapper
+
+
+def format_option(option_text, help_text, required=True):
+ # Maximum column length starting at the option name.
+ COL_LEN = 80
Review comment:
move this to module scope
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,278 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+from .fmtopt import format_option
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
+ARDUINO_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/arduino/template_project"
+
+
+TEMPLATES = {
+ "zephyr": ZEPHYR_TEMPLATE_DIR,
+ "arduino": ARDUINO_TEMPLATE_DIR,
+}
+
+
+@register_parser
+def add_micro_parser(subparsers, main_parser):
+ micro = subparsers.add_parser("micro", help="select micro context.")
+ micro.set_defaults(func=drive_micro)
+
+ micro_parser = micro.add_subparsers(title="subcommands")
+ # Selecting a subcommand under 'micro' is mandatory
+ micro_parser.required = True
+ micro_parser.dest = "subcommand" # options available to select
+
+ # 'create_project' subcommand
+ create_project_parser = micro_parser.add_parser(
+ "create-project", help="create a project template of a given type or given a template dir."
+ )
+ create_project_parser.set_defaults(subcommand_handler=create_project_handler)
+ create_project_parser.add_argument(
+ "PROJECT_DIR",
Review comment:
slight preference for lowercase param names but curious on your take?
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,278 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+from .fmtopt import format_option
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
+ARDUINO_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/arduino/template_project"
+
+
+TEMPLATES = {
+ "zephyr": ZEPHYR_TEMPLATE_DIR,
+ "arduino": ARDUINO_TEMPLATE_DIR,
+}
+
+
+@register_parser
+def add_micro_parser(subparsers, main_parser):
+ micro = subparsers.add_parser("micro", help="select micro context.")
+ micro.set_defaults(func=drive_micro)
+
+ micro_parser = micro.add_subparsers(title="subcommands")
+ # Selecting a subcommand under 'micro' is mandatory
+ micro_parser.required = True
+ micro_parser.dest = "subcommand" # options available to select
+
+ # 'create_project' subcommand
+ create_project_parser = micro_parser.add_parser(
+ "create-project", help="create a project template of a given type or given a template dir."
+ )
+ create_project_parser.set_defaults(subcommand_handler=create_project_handler)
+ create_project_parser.add_argument(
+ "PROJECT_DIR",
+ help="Project dir where the new project based on the template dir will be created.",
+ )
+ create_project_parser.add_argument("MLF", help="MLF .tar archive.")
Review comment:
spell out the acronym in the help
##########
File path: apps/microtvm/zephyr/template_project/microtvm_api_server.py
##########
@@ -229,40 +229,69 @@ def _get_nrf_device_args(options):
PROJECT_OPTIONS = [
server.ProjectOption(
"extra_files_tar",
+ optional=["generate_project"],
+ type="str",
help="If given, during generate_project, uncompress the tarball at this path into the project dir.",
),
server.ProjectOption(
- "gdbserver_port", help=("If given, port number to use when running the local gdbserver.")
+ "gdbserver_port",
+ help=("If given, port number to use when running the local gdbserver."),
+ optional=["open_transport"],
+ type="int",
),
server.ProjectOption(
"nrfjprog_snr",
+ optional=["open_transport"],
+ type="int",
help=("When used with nRF targets, serial # of the attached board to use, from nrfjprog."),
),
server.ProjectOption(
"openocd_serial",
+ optional=["open_transport"],
+ type="int",
help=("When used with OpenOCD targets, serial # of the attached board to use."),
),
server.ProjectOption(
"project_type",
- help="Type of project to generate.",
choices=tuple(PROJECT_TYPES),
+ required=["generate_project"],
+ type="str",
+ help="Type of project to generate.",
+ ),
+ server.ProjectOption(
+ "verbose",
+ optional=["build"],
+ help="Run build with verbose output.",
+ type="bool",
),
- server.ProjectOption("verbose", help="Run build with verbose output.", choices=(True, False)),
server.ProjectOption(
"west_cmd",
+ optional=["generate_project"],
+ default="python3 -m west",
+ type="str",
help=(
"Path to the west tool. If given, supersedes both the zephyr_base "
"option and ZEPHYR_BASE environment variable."
),
),
- server.ProjectOption("zephyr_base", help="Path to the zephyr base directory."),
+ server.ProjectOption(
+ "zephyr_base",
+ optional=["build", "open_transport"],
+ default="ZEPHYR_BASE",
Review comment:
why this default?
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -404,16 +513,24 @@ def run_module(
dev = session.vulkan()
elif device == "rocm":
dev = session.rocm()
+ elif device == "micro":
+ dev = session.device
+ lib = session.get_system_lib()
else:
assert device == "cpu"
dev = session.cpu()
+ # TODO(gromero): Adjust for micro targets.
Review comment:
can you explain more?
##########
File path: src/runtime/rpc/rpc_module.cc
##########
@@ -164,7 +164,7 @@ class RPCModuleNode final : public ModuleNode {
~RPCModuleNode() {
if (module_handle_ != nullptr) {
try {
- sess_->FreeHandle(module_handle_, kTVMModuleHandle);
+ // sess_->FreeHandle(module_handle_, kTVMModuleHandle);
Review comment:
nit: uncomment
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,278 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+from .fmtopt import format_option
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
+ARDUINO_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/arduino/template_project"
+
+
+TEMPLATES = {
+ "zephyr": ZEPHYR_TEMPLATE_DIR,
+ "arduino": ARDUINO_TEMPLATE_DIR,
+}
+
+
+@register_parser
+def add_micro_parser(subparsers, main_parser):
+ micro = subparsers.add_parser("micro", help="select micro context.")
+ micro.set_defaults(func=drive_micro)
+
+ micro_parser = micro.add_subparsers(title="subcommands")
+ # Selecting a subcommand under 'micro' is mandatory
+ micro_parser.required = True
+ micro_parser.dest = "subcommand" # options available to select
+
+ # 'create_project' subcommand
+ create_project_parser = micro_parser.add_parser(
+ "create-project", help="create a project template of a given type or given a template dir."
+ )
+ create_project_parser.set_defaults(subcommand_handler=create_project_handler)
+ create_project_parser.add_argument(
+ "PROJECT_DIR",
+ help="Project dir where the new project based on the template dir will be created.",
+ )
+ create_project_parser.add_argument("MLF", help="MLF .tar archive.")
+ create_project_parser.add_argument(
+ "-f",
+ "--force",
+ action="store_true",
+ help="force project creating even if the specified PROJECT_DIR already exists.",
+ )
+
+ # 'build' subcommand
+ build_parser = micro_parser.add_parser(
+ "build",
+ help="build a project dir, generally creating an image to be flashed, e.g. zephyr.elf.",
+ )
+ build_parser.set_defaults(subcommand_handler=build_handler)
+ build_parser.add_argument("PROJECT_DIR", help="Project dir to build.")
+ build_parser.add_argument("-f", "--force", action="store_true", help="Force rebuild.")
+
+ # 'flash' subcommand
+ flash_parser = micro_parser.add_parser(
+ "flash", help="flash the built image on a given micro target."
+ )
+ flash_parser.set_defaults(subcommand_handler=flash_handler)
+ flash_parser.add_argument("PROJECT_DIR", help="Project dir with a image built.")
+
+ # TODO(gromero): list and select serial when multiple devices exist
+ # It's not clear yet if the device list should be determined by TVMC or returned by the Project API
+
+ # For each platform add arguments detected automatically using Project API info query.
+
+ # Create subparsers for the platforms under 'create-project', 'build', and 'flash' subcommands first.
+ help_msg = "you must selected a platform from the list. You can pass '-h' for a selected platform to list its options."
+ create_project_platforms_parser = create_project_parser.add_subparsers(
+ title="platforms", help=help_msg, dest="platform"
+ )
+ build_platforms_parser = build_parser.add_subparsers(
+ title="platforms", help=help_msg, dest="platform"
+ )
+ flash_platforms_parser = flash_parser.add_subparsers(
+ title="platforms", help=help_msg, dest="platform"
+ )
+
+ # Map used to map a API method to proper CLI parsers and handlers.
+ # API method parser handler smbcmd domain
+ subcmds = {
+ "generate_project": [create_project_platforms_parser, create_project_handler],
+ "build": [build_platforms_parser, build_handler],
+ "flash": [flash_platforms_parser, flash_handler],
+ }
+
+ # Helper to add a platform to a subcmd parser.
+ def _add_parser(parser, platform):
+ platform_name = platform[0].upper() + platform[1:] + " platform"
+ o = parser.add_parser(platform, add_help=False, help=f"select {platform_name}.")
+ o.set_defaults(platform=platform)
+ return o
+
+ parser_by_subcmd = {}
+ for subcmd, o in subcmds.items():
+ parser_by_platform = {}
+ o[0].required = True # Selecting a platform/template is mandatory
+ for platform in TEMPLATES:
+ new_parser = _add_parser(o[0], platform)
+ parser_by_platform[platform] = new_parser
+
+ new_parser = o[0].add_parser("template", add_help=False, help="select an adhoc template.")
+ new_parser.add_argument(
+ "-d", metavar="TEMPLATE_DIR", required=True, help="project API template directory."
Review comment:
can you add `--template-dir` as an option here and then you shouldn't need the metavar?
##########
File path: python/tvm/driver/tvmc/fmtopt.py
##########
@@ -0,0 +1,70 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from textwrap import TextWrapper
+
+
+def format_option(option_text, help_text, required=True):
+ # Maximum column length starting at the option name.
+ COL_LEN = 80
+
+ # Extract option name (option name + '=' char, i.e. '=' is part of what
+ # is considered here 'optname') plus its length for choices and help
+ # text justification.
+ optname_break = option_text.find("=")
+ optname = option_text[: optname_break + 1] # '=' is part of the option name in this context
+ optname_len = len(optname)
+
+ # Prepare optname + choices text chunck.
+ choices_text = option_text[optname_break + 1 :] # '=' is not part of the choices
+ wrapper = TextWrapper(width=COL_LEN - optname_len)
+ choices_lines = wrapper.wrap(choices_text)
+
+ # Set first choices line which merely appends to optname string.
+ # No justification is necessary for the first line since first
+ # line was wrapped based on COL_LEN - optname_len, i.e. considering
+ # optname_len.
+ choices_just_lines = [optname + choices_lines[0]]
+
+ # Justify the remaining lines based on first optname + '='.
+ for line in choices_lines[1:]:
+ line_len = len(line)
+ line_just = line.rjust(
+ optname_len + 1 + line_len
+ ) # add 1 to align after '{' in the line above
+ choices_just_lines.append(line_just)
+
+ choices_text_just_chunk = "\n".join(choices_just_lines)
+
+ # Prepare help text chunck.
+ wrapper = TextWrapper(width=120)
Review comment:
why 120 here?
##########
File path: python/tvm/micro/session.py
##########
@@ -107,7 +107,7 @@ def _wrap_transport_write(self, data, timeout_microsec):
return len(data) # TODO(areusch): delete
- def __enter__(self):
+ def open(self):
Review comment:
seems like open should return nothing and __enter__ should return self
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -366,13 +460,17 @@ def run_module(
"Try calling tvmc.compile on the model before running it."
)
- # Currently only two package formats are supported: "classic" and
- # "mlf". The later can only be used for micro targets, i.e. with microTVM.
- if tvmc_package.type == "mlf":
- raise TVMCException(
- "You're trying to run a model saved using the Model Library Format (MLF)."
- "MLF can only be used to run micro targets (microTVM)."
- )
+ micro = False
+ if device == "micro":
+ if tvmc_package.type != "mlf":
+ TVMCException("--device 'micro' specified but no MLF archive found in PATH.")
Review comment:
i'd phrase it more in terms of which command-line arg is missing
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -97,7 +111,55 @@ def add_run_parser(subparsers):
help="hostname (required) and port (optional, defaults to 9090) of the RPC tracker, "
"e.g. '192.168.0.100:9999'",
)
- parser.add_argument("FILE", help="path to the compiled module file")
+ parser.add_argument(
+ "PATH",
+ help="path to the compiled module file or to the project directory (micro devices only).",
+ )
+
+ #
+ # Hack to lookahead if 'run' was selected and make
+ # two dynamic parsers (in micro.py and in runner.py) work.
+ #
+ # print(sys.argv)
+ if "run" in sys.argv:
Review comment:
suggest to invert the condition and return early. also, what if `run` is e.g. the value of --template-dir?
--
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] areusch commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r727495939
##########
File path: apps/microtvm/zephyr/template_project/microtvm_api_server.py
##########
@@ -229,40 +229,69 @@ def _get_nrf_device_args(options):
PROJECT_OPTIONS = [
server.ProjectOption(
"extra_files_tar",
+ optional=["generate_project"],
+ type="str",
help="If given, during generate_project, uncompress the tarball at this path into the project dir.",
),
server.ProjectOption(
- "gdbserver_port", help=("If given, port number to use when running the local gdbserver.")
+ "gdbserver_port",
+ help=("If given, port number to use when running the local gdbserver."),
+ optional=["open_transport"],
+ type="int",
),
server.ProjectOption(
"nrfjprog_snr",
+ optional=["open_transport"],
+ type="int",
help=("When used with nRF targets, serial # of the attached board to use, from nrfjprog."),
),
server.ProjectOption(
"openocd_serial",
+ optional=["open_transport"],
+ type="int",
help=("When used with OpenOCD targets, serial # of the attached board to use."),
),
server.ProjectOption(
"project_type",
- help="Type of project to generate.",
choices=tuple(PROJECT_TYPES),
+ required=["generate_project"],
+ type="str",
+ help="Type of project to generate.",
+ ),
+ server.ProjectOption(
+ "verbose",
+ optional=["build"],
+ help="Run build with verbose output.",
+ type="bool",
),
- server.ProjectOption("verbose", help="Run build with verbose output.", choices=(True, False)),
server.ProjectOption(
"west_cmd",
+ optional=["generate_project"],
+ default="python3 -m west",
Review comment:
is this needed? prefer `sys.executable` if possible
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -520,3 +522,84 @@ def parse_configs(input_configs):
pass_context_configs[name] = parsed_value
return pass_context_configs
+
+
+def get_project_options(project_info):
Review comment:
can you add docstring and type annotation if possible?
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,278 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+from .fmtopt import format_option
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
+ARDUINO_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/arduino/template_project"
+
+
+TEMPLATES = {
+ "zephyr": ZEPHYR_TEMPLATE_DIR,
+ "arduino": ARDUINO_TEMPLATE_DIR,
+}
+
+
+@register_parser
+def add_micro_parser(subparsers, main_parser):
+ micro = subparsers.add_parser("micro", help="select micro context.")
+ micro.set_defaults(func=drive_micro)
+
+ micro_parser = micro.add_subparsers(title="subcommands")
+ # Selecting a subcommand under 'micro' is mandatory
+ micro_parser.required = True
+ micro_parser.dest = "subcommand" # options available to select
+
+ # 'create_project' subcommand
+ create_project_parser = micro_parser.add_parser(
+ "create-project", help="create a project template of a given type or given a template dir."
+ )
+ create_project_parser.set_defaults(subcommand_handler=create_project_handler)
+ create_project_parser.add_argument(
+ "PROJECT_DIR",
+ help="Project dir where the new project based on the template dir will be created.",
+ )
+ create_project_parser.add_argument("MLF", help="MLF .tar archive.")
+ create_project_parser.add_argument(
+ "-f",
+ "--force",
+ action="store_true",
+ help="force project creating even if the specified PROJECT_DIR already exists.",
+ )
+
+ # 'build' subcommand
+ build_parser = micro_parser.add_parser(
+ "build",
+ help="build a project dir, generally creating an image to be flashed, e.g. zephyr.elf.",
+ )
+ build_parser.set_defaults(subcommand_handler=build_handler)
+ build_parser.add_argument("PROJECT_DIR", help="Project dir to build.")
+ build_parser.add_argument("-f", "--force", action="store_true", help="Force rebuild.")
+
+ # 'flash' subcommand
+ flash_parser = micro_parser.add_parser(
+ "flash", help="flash the built image on a given micro target."
+ )
+ flash_parser.set_defaults(subcommand_handler=flash_handler)
+ flash_parser.add_argument("PROJECT_DIR", help="Project dir with a image built.")
+
+ # TODO(gromero): list and select serial when multiple devices exist
+ # It's not clear yet if the device list should be determined by TVMC or returned by the Project API
+
+ # For each platform add arguments detected automatically using Project API info query.
+
+ # Create subparsers for the platforms under 'create-project', 'build', and 'flash' subcommands first.
+ help_msg = "you must selected a platform from the list. You can pass '-h' for a selected platform to list its options."
+ create_project_platforms_parser = create_project_parser.add_subparsers(
+ title="platforms", help=help_msg, dest="platform"
+ )
+ build_platforms_parser = build_parser.add_subparsers(
+ title="platforms", help=help_msg, dest="platform"
+ )
+ flash_platforms_parser = flash_parser.add_subparsers(
+ title="platforms", help=help_msg, dest="platform"
+ )
+
+ # Map used to map a API method to proper CLI parsers and handlers.
+ # API method parser handler smbcmd domain
+ subcmds = {
+ "generate_project": [create_project_platforms_parser, create_project_handler],
+ "build": [build_platforms_parser, build_handler],
+ "flash": [flash_platforms_parser, flash_handler],
+ }
+
+ # Helper to add a platform to a subcmd parser.
+ def _add_parser(parser, platform):
+ platform_name = platform[0].upper() + platform[1:] + " platform"
+ o = parser.add_parser(platform, add_help=False, help=f"select {platform_name}.")
+ o.set_defaults(platform=platform)
+ return o
+
+ parser_by_subcmd = {}
+ for subcmd, o in subcmds.items():
+ parser_by_platform = {}
+ o[0].required = True # Selecting a platform/template is mandatory
+ for platform in TEMPLATES:
+ new_parser = _add_parser(o[0], platform)
+ parser_by_platform[platform] = new_parser
+
+ new_parser = o[0].add_parser("template", add_help=False, help="select an adhoc template.")
+ new_parser.add_argument(
+ "-d", metavar="TEMPLATE_DIR", required=True, help="project API template directory."
+ )
+ new_parser.set_defaults(platform="template")
+ parser_by_platform["template"] = new_parser
+
+ parser_by_subcmd[subcmd] = parser_by_platform
+
+ known_args, unkown_args = main_parser.parse_known_args()
+
+ try:
+ subcmd = known_args.subcommand
+ platform = known_args.platform
+ except AttributeError:
+ # No subcommand or platform, so not need to augment the parser.
+ return
+
+ # Augment parser with project options
+
+ if platform == "template":
+ # adhoc template
+ template_dir = known_args.d
+ else:
+ # default template
+ template_dir = TEMPLATES[platform]
+
+ template = project.TemplateProject.from_directory(template_dir)
+ template_info = template.info()
+
+ options_by_method = get_project_options(template_info)
+
+ # TODO(gromero): refactor to remove this map.
+ subcmd_to_method = {
+ "create-project": "generate_project",
+ "build": "build",
+ "flash": "flash",
+ }
+
+ method = subcmd_to_method[subcmd]
+ parser_by_subcmd_n_platform = parser_by_subcmd[method][platform]
+ _, handler = subcmds[method]
+
+ parser_by_subcmd_n_platform.formatter_class = (
+ argparse.RawTextHelpFormatter
+ ) # Set raw help text so help_text format works
+ parser_by_subcmd_n_platform.set_defaults(
+ subcommand_handler=handler,
+ valid_options=options_by_method[method],
+ template_dir=template_dir,
+ )
+
+ parser_by_subcmd_n_platform.add_argument(
+ "--list-options",
+ action="help",
+ help="show all options/values for selected platforms/template.",
+ )
+
+ required = any([opt["required"] for opt in options_by_method[method]])
+ nargs = "+" if required else "*"
+
+ help_text_by_option = [opt["help_text"] for opt in options_by_method[method]]
+ help_text = "\n\n".join(help_text_by_option) + "\n\n"
+
+ # TODO(gromero): Experiment with required=required below
+ parser_by_subcmd_n_platform.add_argument(
+ "-o", required=True, metavar="OPTION=VALUE", nargs=nargs, help=help_text
Review comment:
i might suggest adding --project-option as an arg name to keep the code more readable
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -520,3 +522,84 @@ def parse_configs(input_configs):
pass_context_configs[name] = parsed_value
return pass_context_configs
+
+
+def get_project_options(project_info):
+ options = project_info["project_options"]
+
+ options_by_method = defaultdict(list)
+ for opt in options:
+ # Get list of methods associated with an option based on the
+ # existance of a 'required' or 'optional' lists. API specification
+ # guarantees at least one of these lists will exist. If a list does
+ # not exist it's returned as None by the API.
+ metadata = ["required", "optional"]
+ om = [(opt[md], True if md == "required" else False) for md in metadata if opt[md]]
+ for methods, is_opt_required in om:
+ for method in methods:
+ name = opt["name"]
+
+ if opt["type"] == "bool":
+ # TODO(gromero): Use only choices= and merge with non-bool options below
+ option_choices_text = f"{name}={{true, false}}"
Review comment:
it might be a bit cleaner to just set `choices = ["true", "false"]` and reuse the logic below.
##########
File path: apps/microtvm/zephyr/template_project/microtvm_api_server.py
##########
@@ -229,40 +229,69 @@ def _get_nrf_device_args(options):
PROJECT_OPTIONS = [
server.ProjectOption(
"extra_files_tar",
+ optional=["generate_project"],
+ type="str",
Review comment:
i think use just `str` not `"str"` here and elsewhere
##########
File path: python/tvm/driver/tvmc/fmtopt.py
##########
@@ -0,0 +1,70 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from textwrap import TextWrapper
+
+
+def format_option(option_text, help_text, required=True):
+ # Maximum column length starting at the option name.
+ COL_LEN = 80
+
+ # Extract option name (option name + '=' char, i.e. '=' is part of what
+ # is considered here 'optname') plus its length for choices and help
+ # text justification.
+ optname_break = option_text.find("=")
Review comment:
cleaner: `optname, chioces_text = option_text.split("=", 1)`
##########
File path: python/tvm/driver/tvmc/fmtopt.py
##########
@@ -0,0 +1,70 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from textwrap import TextWrapper
+
+
+def format_option(option_text, help_text, required=True):
+ # Maximum column length starting at the option name.
+ COL_LEN = 80
Review comment:
move this to module scope
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,278 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+from .fmtopt import format_option
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
+ARDUINO_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/arduino/template_project"
+
+
+TEMPLATES = {
+ "zephyr": ZEPHYR_TEMPLATE_DIR,
+ "arduino": ARDUINO_TEMPLATE_DIR,
+}
+
+
+@register_parser
+def add_micro_parser(subparsers, main_parser):
+ micro = subparsers.add_parser("micro", help="select micro context.")
+ micro.set_defaults(func=drive_micro)
+
+ micro_parser = micro.add_subparsers(title="subcommands")
+ # Selecting a subcommand under 'micro' is mandatory
+ micro_parser.required = True
+ micro_parser.dest = "subcommand" # options available to select
+
+ # 'create_project' subcommand
+ create_project_parser = micro_parser.add_parser(
+ "create-project", help="create a project template of a given type or given a template dir."
+ )
+ create_project_parser.set_defaults(subcommand_handler=create_project_handler)
+ create_project_parser.add_argument(
+ "PROJECT_DIR",
Review comment:
slight preference for lowercase param names but curious on your take?
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,278 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+from .fmtopt import format_option
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
+ARDUINO_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/arduino/template_project"
+
+
+TEMPLATES = {
+ "zephyr": ZEPHYR_TEMPLATE_DIR,
+ "arduino": ARDUINO_TEMPLATE_DIR,
+}
+
+
+@register_parser
+def add_micro_parser(subparsers, main_parser):
+ micro = subparsers.add_parser("micro", help="select micro context.")
+ micro.set_defaults(func=drive_micro)
+
+ micro_parser = micro.add_subparsers(title="subcommands")
+ # Selecting a subcommand under 'micro' is mandatory
+ micro_parser.required = True
+ micro_parser.dest = "subcommand" # options available to select
+
+ # 'create_project' subcommand
+ create_project_parser = micro_parser.add_parser(
+ "create-project", help="create a project template of a given type or given a template dir."
+ )
+ create_project_parser.set_defaults(subcommand_handler=create_project_handler)
+ create_project_parser.add_argument(
+ "PROJECT_DIR",
+ help="Project dir where the new project based on the template dir will be created.",
+ )
+ create_project_parser.add_argument("MLF", help="MLF .tar archive.")
Review comment:
spell out the acronym in the help
##########
File path: apps/microtvm/zephyr/template_project/microtvm_api_server.py
##########
@@ -229,40 +229,69 @@ def _get_nrf_device_args(options):
PROJECT_OPTIONS = [
server.ProjectOption(
"extra_files_tar",
+ optional=["generate_project"],
+ type="str",
help="If given, during generate_project, uncompress the tarball at this path into the project dir.",
),
server.ProjectOption(
- "gdbserver_port", help=("If given, port number to use when running the local gdbserver.")
+ "gdbserver_port",
+ help=("If given, port number to use when running the local gdbserver."),
+ optional=["open_transport"],
+ type="int",
),
server.ProjectOption(
"nrfjprog_snr",
+ optional=["open_transport"],
+ type="int",
help=("When used with nRF targets, serial # of the attached board to use, from nrfjprog."),
),
server.ProjectOption(
"openocd_serial",
+ optional=["open_transport"],
+ type="int",
help=("When used with OpenOCD targets, serial # of the attached board to use."),
),
server.ProjectOption(
"project_type",
- help="Type of project to generate.",
choices=tuple(PROJECT_TYPES),
+ required=["generate_project"],
+ type="str",
+ help="Type of project to generate.",
+ ),
+ server.ProjectOption(
+ "verbose",
+ optional=["build"],
+ help="Run build with verbose output.",
+ type="bool",
),
- server.ProjectOption("verbose", help="Run build with verbose output.", choices=(True, False)),
server.ProjectOption(
"west_cmd",
+ optional=["generate_project"],
+ default="python3 -m west",
+ type="str",
help=(
"Path to the west tool. If given, supersedes both the zephyr_base "
"option and ZEPHYR_BASE environment variable."
),
),
- server.ProjectOption("zephyr_base", help="Path to the zephyr base directory."),
+ server.ProjectOption(
+ "zephyr_base",
+ optional=["build", "open_transport"],
+ default="ZEPHYR_BASE",
Review comment:
why this default?
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -404,16 +513,24 @@ def run_module(
dev = session.vulkan()
elif device == "rocm":
dev = session.rocm()
+ elif device == "micro":
+ dev = session.device
+ lib = session.get_system_lib()
else:
assert device == "cpu"
dev = session.cpu()
+ # TODO(gromero): Adjust for micro targets.
Review comment:
can you explain more?
##########
File path: src/runtime/rpc/rpc_module.cc
##########
@@ -164,7 +164,7 @@ class RPCModuleNode final : public ModuleNode {
~RPCModuleNode() {
if (module_handle_ != nullptr) {
try {
- sess_->FreeHandle(module_handle_, kTVMModuleHandle);
+ // sess_->FreeHandle(module_handle_, kTVMModuleHandle);
Review comment:
nit: uncomment
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,278 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+from .fmtopt import format_option
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
+ARDUINO_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/arduino/template_project"
+
+
+TEMPLATES = {
+ "zephyr": ZEPHYR_TEMPLATE_DIR,
+ "arduino": ARDUINO_TEMPLATE_DIR,
+}
+
+
+@register_parser
+def add_micro_parser(subparsers, main_parser):
+ micro = subparsers.add_parser("micro", help="select micro context.")
+ micro.set_defaults(func=drive_micro)
+
+ micro_parser = micro.add_subparsers(title="subcommands")
+ # Selecting a subcommand under 'micro' is mandatory
+ micro_parser.required = True
+ micro_parser.dest = "subcommand" # options available to select
+
+ # 'create_project' subcommand
+ create_project_parser = micro_parser.add_parser(
+ "create-project", help="create a project template of a given type or given a template dir."
+ )
+ create_project_parser.set_defaults(subcommand_handler=create_project_handler)
+ create_project_parser.add_argument(
+ "PROJECT_DIR",
+ help="Project dir where the new project based on the template dir will be created.",
+ )
+ create_project_parser.add_argument("MLF", help="MLF .tar archive.")
+ create_project_parser.add_argument(
+ "-f",
+ "--force",
+ action="store_true",
+ help="force project creating even if the specified PROJECT_DIR already exists.",
+ )
+
+ # 'build' subcommand
+ build_parser = micro_parser.add_parser(
+ "build",
+ help="build a project dir, generally creating an image to be flashed, e.g. zephyr.elf.",
+ )
+ build_parser.set_defaults(subcommand_handler=build_handler)
+ build_parser.add_argument("PROJECT_DIR", help="Project dir to build.")
+ build_parser.add_argument("-f", "--force", action="store_true", help="Force rebuild.")
+
+ # 'flash' subcommand
+ flash_parser = micro_parser.add_parser(
+ "flash", help="flash the built image on a given micro target."
+ )
+ flash_parser.set_defaults(subcommand_handler=flash_handler)
+ flash_parser.add_argument("PROJECT_DIR", help="Project dir with a image built.")
+
+ # TODO(gromero): list and select serial when multiple devices exist
+ # It's not clear yet if the device list should be determined by TVMC or returned by the Project API
+
+ # For each platform add arguments detected automatically using Project API info query.
+
+ # Create subparsers for the platforms under 'create-project', 'build', and 'flash' subcommands first.
+ help_msg = "you must selected a platform from the list. You can pass '-h' for a selected platform to list its options."
+ create_project_platforms_parser = create_project_parser.add_subparsers(
+ title="platforms", help=help_msg, dest="platform"
+ )
+ build_platforms_parser = build_parser.add_subparsers(
+ title="platforms", help=help_msg, dest="platform"
+ )
+ flash_platforms_parser = flash_parser.add_subparsers(
+ title="platforms", help=help_msg, dest="platform"
+ )
+
+ # Map used to map a API method to proper CLI parsers and handlers.
+ # API method parser handler smbcmd domain
+ subcmds = {
+ "generate_project": [create_project_platforms_parser, create_project_handler],
+ "build": [build_platforms_parser, build_handler],
+ "flash": [flash_platforms_parser, flash_handler],
+ }
+
+ # Helper to add a platform to a subcmd parser.
+ def _add_parser(parser, platform):
+ platform_name = platform[0].upper() + platform[1:] + " platform"
+ o = parser.add_parser(platform, add_help=False, help=f"select {platform_name}.")
+ o.set_defaults(platform=platform)
+ return o
+
+ parser_by_subcmd = {}
+ for subcmd, o in subcmds.items():
+ parser_by_platform = {}
+ o[0].required = True # Selecting a platform/template is mandatory
+ for platform in TEMPLATES:
+ new_parser = _add_parser(o[0], platform)
+ parser_by_platform[platform] = new_parser
+
+ new_parser = o[0].add_parser("template", add_help=False, help="select an adhoc template.")
+ new_parser.add_argument(
+ "-d", metavar="TEMPLATE_DIR", required=True, help="project API template directory."
Review comment:
can you add `--template-dir` as an option here and then you shouldn't need the metavar?
##########
File path: python/tvm/driver/tvmc/fmtopt.py
##########
@@ -0,0 +1,70 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from textwrap import TextWrapper
+
+
+def format_option(option_text, help_text, required=True):
+ # Maximum column length starting at the option name.
+ COL_LEN = 80
+
+ # Extract option name (option name + '=' char, i.e. '=' is part of what
+ # is considered here 'optname') plus its length for choices and help
+ # text justification.
+ optname_break = option_text.find("=")
+ optname = option_text[: optname_break + 1] # '=' is part of the option name in this context
+ optname_len = len(optname)
+
+ # Prepare optname + choices text chunck.
+ choices_text = option_text[optname_break + 1 :] # '=' is not part of the choices
+ wrapper = TextWrapper(width=COL_LEN - optname_len)
+ choices_lines = wrapper.wrap(choices_text)
+
+ # Set first choices line which merely appends to optname string.
+ # No justification is necessary for the first line since first
+ # line was wrapped based on COL_LEN - optname_len, i.e. considering
+ # optname_len.
+ choices_just_lines = [optname + choices_lines[0]]
+
+ # Justify the remaining lines based on first optname + '='.
+ for line in choices_lines[1:]:
+ line_len = len(line)
+ line_just = line.rjust(
+ optname_len + 1 + line_len
+ ) # add 1 to align after '{' in the line above
+ choices_just_lines.append(line_just)
+
+ choices_text_just_chunk = "\n".join(choices_just_lines)
+
+ # Prepare help text chunck.
+ wrapper = TextWrapper(width=120)
Review comment:
why 120 here?
##########
File path: python/tvm/micro/session.py
##########
@@ -107,7 +107,7 @@ def _wrap_transport_write(self, data, timeout_microsec):
return len(data) # TODO(areusch): delete
- def __enter__(self):
+ def open(self):
Review comment:
seems like open should return nothing and __enter__ should return self
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -366,13 +460,17 @@ def run_module(
"Try calling tvmc.compile on the model before running it."
)
- # Currently only two package formats are supported: "classic" and
- # "mlf". The later can only be used for micro targets, i.e. with microTVM.
- if tvmc_package.type == "mlf":
- raise TVMCException(
- "You're trying to run a model saved using the Model Library Format (MLF)."
- "MLF can only be used to run micro targets (microTVM)."
- )
+ micro = False
+ if device == "micro":
+ if tvmc_package.type != "mlf":
+ TVMCException("--device 'micro' specified but no MLF archive found in PATH.")
Review comment:
i'd phrase it more in terms of which command-line arg is missing
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -97,7 +111,55 @@ def add_run_parser(subparsers):
help="hostname (required) and port (optional, defaults to 9090) of the RPC tracker, "
"e.g. '192.168.0.100:9999'",
)
- parser.add_argument("FILE", help="path to the compiled module file")
+ parser.add_argument(
+ "PATH",
+ help="path to the compiled module file or to the project directory (micro devices only).",
+ )
+
+ #
+ # Hack to lookahead if 'run' was selected and make
+ # two dynamic parsers (in micro.py and in runner.py) work.
+ #
+ # print(sys.argv)
+ if "run" in sys.argv:
Review comment:
suggest to invert the condition and return early. also, what if `run` is e.g. the value of --template-dir?
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735801323
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,277 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
Review comment:
@mehrdadh Thanks for the heads up. I'll keep and eye on it and take care to rebase once it's merged.
--
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] mehrdadh commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735924029
##########
File path: apps/microtvm/zephyr/template_project/microtvm_api_server.py
##########
@@ -229,40 +229,69 @@ def _get_nrf_device_args(options):
PROJECT_OPTIONS = [
server.ProjectOption(
"extra_files_tar",
+ optional=["generate_project"],
+ type="str",
help="If given, during generate_project, uncompress the tarball at this path into the project dir.",
),
server.ProjectOption(
- "gdbserver_port", help=("If given, port number to use when running the local gdbserver.")
+ "gdbserver_port",
+ help=("If given, port number to use when running the local gdbserver."),
+ optional=["open_transport"],
+ type="int",
),
server.ProjectOption(
"nrfjprog_snr",
+ optional=["open_transport"],
+ type="int",
help=("When used with nRF targets, serial # of the attached board to use, from nrfjprog."),
),
server.ProjectOption(
"openocd_serial",
+ optional=["open_transport"],
+ type="int",
help=("When used with OpenOCD targets, serial # of the attached board to use."),
),
server.ProjectOption(
"project_type",
- help="Type of project to generate.",
choices=tuple(PROJECT_TYPES),
+ required=["generate_project"],
+ type="str",
+ help="Type of project to generate.",
+ ),
+ server.ProjectOption(
+ "verbose",
+ optional=["build"],
+ help="Run build with verbose output.",
+ type="bool",
),
- server.ProjectOption("verbose", help="Run build with verbose output.", choices=(True, False)),
server.ProjectOption(
"west_cmd",
+ optional=["generate_project"],
+ default="python3 -m west",
+ type="str",
help=(
"Path to the west tool. If given, supersedes both the zephyr_base "
"option and ZEPHYR_BASE environment variable."
),
),
- server.ProjectOption("zephyr_base", help="Path to the zephyr base directory."),
+ server.ProjectOption(
+ "zephyr_base",
+ optional=["build", "open_transport"],
+ default="ZEPHYR_BASE",
Review comment:
looks good.
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735726417
##########
File path: apps/microtvm/zephyr/template_project/microtvm_api_server.py
##########
@@ -229,40 +229,69 @@ def _get_nrf_device_args(options):
PROJECT_OPTIONS = [
server.ProjectOption(
"extra_files_tar",
+ optional=["generate_project"],
+ type="str",
help="If given, during generate_project, uncompress the tarball at this path into the project dir.",
),
server.ProjectOption(
- "gdbserver_port", help=("If given, port number to use when running the local gdbserver.")
+ "gdbserver_port",
+ help=("If given, port number to use when running the local gdbserver."),
+ optional=["open_transport"],
+ type="int",
),
server.ProjectOption(
"nrfjprog_snr",
+ optional=["open_transport"],
+ type="int",
help=("When used with nRF targets, serial # of the attached board to use, from nrfjprog."),
),
server.ProjectOption(
"openocd_serial",
+ optional=["open_transport"],
+ type="int",
help=("When used with OpenOCD targets, serial # of the attached board to use."),
),
server.ProjectOption(
"project_type",
- help="Type of project to generate.",
choices=tuple(PROJECT_TYPES),
+ required=["generate_project"],
+ type="str",
+ help="Type of project to generate.",
+ ),
+ server.ProjectOption(
+ "verbose",
+ optional=["build"],
+ help="Run build with verbose output.",
+ type="bool",
),
- server.ProjectOption("verbose", help="Run build with verbose output.", choices=(True, False)),
server.ProjectOption(
"west_cmd",
+ optional=["generate_project"],
+ default="python3 -m west",
Review comment:
I was not totally sure at first, but I thought it would be kind to let the user known what the default is here.
I've changed it as you suggested and now it uses `sys.executable`. So, done in https://github.com/apache/tvm/pull/9229/commits/efbc2803a2f5c4775a4d6c0019e3ce0d3e262718
I also took the chance to add `default` value to the help message in https://github.com/apache/tvm/pull/9229/commits/9bf6643bd083a006034b87067d4f3a192c7287fe so now, for instance, `west_cmd` is displayed to the user like:
```
west_cmd=WEST_CMD
path to the west tool. If given, supersedes both the zephyr_base option and ZEPHYR_BASE environment variable. Defaults to '/usr/bin/python3 -m west'.
```
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735769188
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -366,13 +460,17 @@ def run_module(
"Try calling tvmc.compile on the model before running it."
)
- # Currently only two package formats are supported: "classic" and
- # "mlf". The later can only be used for micro targets, i.e. with microTVM.
- if tvmc_package.type == "mlf":
- raise TVMCException(
- "You're trying to run a model saved using the Model Library Format (MLF)."
- "MLF can only be used to run micro targets (microTVM)."
- )
+ micro = False
+ if device == "micro":
+ if tvmc_package.type != "mlf":
+ TVMCException("--device 'micro' specified but no MLF archive found in PATH.")
Review comment:
Actually no command line is missing. This is catching the case when a `model.tar` file is found in the project dir but it is a `classic` format, not a MLF. I've tried to improve the message a bit anyway. Done in https://github.com/apache/tvm/pull/9229/commits/8f6f6f682d40dd8a1b0c2cb8206956d81517663b
I also realized that I was not effectively calling `raise` there so no exception was raised. I fixed that also in the commit above.
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735795280
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -520,3 +522,84 @@ def parse_configs(input_configs):
pass_context_configs[name] = parsed_value
return pass_context_configs
+
+
+def get_project_options(project_info):
+ options = project_info["project_options"]
+
+ options_by_method = defaultdict(list)
+ for opt in options:
+ # Get list of methods associated with an option based on the
+ # existance of a 'required' or 'optional' lists. API specification
+ # guarantees at least one of these lists will exist. If a list does
+ # not exist it's returned as None by the API.
+ metadata = ["required", "optional"]
+ om = [(opt[md], True if md == "required" else False) for md in metadata if opt[md]]
+ for methods, is_opt_required in om:
+ for method in methods:
+ name = opt["name"]
+
+ if opt["type"] == "bool":
+ # TODO(gromero): Use only choices= and merge with non-bool options below
+ option_choices_text = f"{name}={{true, false}}"
+
+ else:
+ if opt["choices"]:
+ choices = "{" + ", ".join(opt["choices"]) + "}"
+ else:
+ choices = opt["name"].upper()
+
+ option_choices_text = f"{name}={choices}"
+
+ help_text = opt["help"][0].lower() + opt["help"][1:]
+ formatted_help_text = format_option(option_choices_text, help_text, is_opt_required)
+
+ option = {
+ "name": opt["name"],
+ "choices": opt["choices"],
+ "help_text": formatted_help_text,
+ "required": is_opt_required,
+ }
+ options_by_method[method].append(option)
+
+ return options_by_method
+
+
+def get_options(options):
Review comment:
@mehrdadh added docstrings for `get_options` and other functions in https://github.com/apache/tvm/pull/9229/commits/7ed6635ba3f05c7ab9cbd319986b30ff9de63990
https://github.com/apache/tvm/pull/9229/commits/2ff3d802540be393820e7f3e8af64b47a795058e
https://github.com/apache/tvm/pull/9229/commits/377174558bd3adbf2f0ca4fbc00bec0abd9a1bb6
https://github.com/apache/tvm/pull/9229/commits/7534618bd051d372f57e6469c933bac66196fb94
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -520,3 +522,84 @@ def parse_configs(input_configs):
pass_context_configs[name] = parsed_value
return pass_context_configs
+
+
+def get_project_options(project_info):
+ options = project_info["project_options"]
+
+ options_by_method = defaultdict(list)
+ for opt in options:
+ # Get list of methods associated with an option based on the
+ # existance of a 'required' or 'optional' lists. API specification
+ # guarantees at least one of these lists will exist. If a list does
+ # not exist it's returned as None by the API.
+ metadata = ["required", "optional"]
+ om = [(opt[md], True if md == "required" else False) for md in metadata if opt[md]]
+ for methods, is_opt_required in om:
+ for method in methods:
+ name = opt["name"]
+
+ if opt["type"] == "bool":
+ # TODO(gromero): Use only choices= and merge with non-bool options below
+ option_choices_text = f"{name}={{true, false}}"
+
+ else:
+ if opt["choices"]:
+ choices = "{" + ", ".join(opt["choices"]) + "}"
+ else:
+ choices = opt["name"].upper()
+
+ option_choices_text = f"{name}={choices}"
+
+ help_text = opt["help"][0].lower() + opt["help"][1:]
+ formatted_help_text = format_option(option_choices_text, help_text, is_opt_required)
+
+ option = {
+ "name": opt["name"],
+ "choices": opt["choices"],
+ "help_text": formatted_help_text,
+ "required": is_opt_required,
+ }
+ options_by_method[method].append(option)
+
+ return options_by_method
+
+
+def get_options(options):
Review comment:
@mehrdadh added docstrings for `get_options` and other functions in
https://github.com/apache/tvm/pull/9229/commits/7ed6635ba3f05c7ab9cbd319986b30ff9de63990
https://github.com/apache/tvm/pull/9229/commits/2ff3d802540be393820e7f3e8af64b47a795058e
https://github.com/apache/tvm/pull/9229/commits/377174558bd3adbf2f0ca4fbc00bec0abd9a1bb6
https://github.com/apache/tvm/pull/9229/commits/7534618bd051d372f57e6469c933bac66196fb94
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735741501
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,278 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+from .fmtopt import format_option
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
+ARDUINO_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/arduino/template_project"
+
+
+TEMPLATES = {
+ "zephyr": ZEPHYR_TEMPLATE_DIR,
+ "arduino": ARDUINO_TEMPLATE_DIR,
+}
+
+
+@register_parser
+def add_micro_parser(subparsers, main_parser):
+ micro = subparsers.add_parser("micro", help="select micro context.")
+ micro.set_defaults(func=drive_micro)
+
+ micro_parser = micro.add_subparsers(title="subcommands")
+ # Selecting a subcommand under 'micro' is mandatory
+ micro_parser.required = True
+ micro_parser.dest = "subcommand" # options available to select
+
+ # 'create_project' subcommand
+ create_project_parser = micro_parser.add_parser(
+ "create-project", help="create a project template of a given type or given a template dir."
+ )
+ create_project_parser.set_defaults(subcommand_handler=create_project_handler)
+ create_project_parser.add_argument(
+ "PROJECT_DIR",
Review comment:
I'm ok with lowercase. Done in https://github.com/apache/tvm/pull/9229/commits/0faa00ebba9e13b50d17059fb67c66751ece4ab9
--
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] gromero commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r735742140
##########
File path: python/tvm/driver/tvmc/micro.py
##########
@@ -0,0 +1,278 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import shutil
+from pathlib import Path
+from collections import defaultdict
+import argparse
+
+import tvm.micro.project as project
+from tvm.micro.project_api.server import ServerError
+from .main import register_parser
+from .common import (
+ TVMCException,
+ get_project_options,
+ get_options,
+ check_options,
+ check_options_choices,
+)
+from .fmtopt import format_option
+
+
+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"
+ARDUINO_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/arduino/template_project"
+
+
+TEMPLATES = {
+ "zephyr": ZEPHYR_TEMPLATE_DIR,
+ "arduino": ARDUINO_TEMPLATE_DIR,
+}
+
+
+@register_parser
+def add_micro_parser(subparsers, main_parser):
+ micro = subparsers.add_parser("micro", help="select micro context.")
+ micro.set_defaults(func=drive_micro)
+
+ micro_parser = micro.add_subparsers(title="subcommands")
+ # Selecting a subcommand under 'micro' is mandatory
+ micro_parser.required = True
+ micro_parser.dest = "subcommand" # options available to select
+
+ # 'create_project' subcommand
+ create_project_parser = micro_parser.add_parser(
+ "create-project", help="create a project template of a given type or given a template dir."
+ )
+ create_project_parser.set_defaults(subcommand_handler=create_project_handler)
+ create_project_parser.add_argument(
+ "PROJECT_DIR",
+ help="Project dir where the new project based on the template dir will be created.",
+ )
+ create_project_parser.add_argument("MLF", help="MLF .tar archive.")
+ create_project_parser.add_argument(
+ "-f",
+ "--force",
+ action="store_true",
+ help="force project creating even if the specified PROJECT_DIR already exists.",
+ )
+
+ # 'build' subcommand
+ build_parser = micro_parser.add_parser(
+ "build",
+ help="build a project dir, generally creating an image to be flashed, e.g. zephyr.elf.",
+ )
+ build_parser.set_defaults(subcommand_handler=build_handler)
+ build_parser.add_argument("PROJECT_DIR", help="Project dir to build.")
+ build_parser.add_argument("-f", "--force", action="store_true", help="Force rebuild.")
+
+ # 'flash' subcommand
+ flash_parser = micro_parser.add_parser(
+ "flash", help="flash the built image on a given micro target."
+ )
+ flash_parser.set_defaults(subcommand_handler=flash_handler)
+ flash_parser.add_argument("PROJECT_DIR", help="Project dir with a image built.")
+
+ # TODO(gromero): list and select serial when multiple devices exist
+ # It's not clear yet if the device list should be determined by TVMC or returned by the Project API
+
+ # For each platform add arguments detected automatically using Project API info query.
+
+ # Create subparsers for the platforms under 'create-project', 'build', and 'flash' subcommands first.
+ help_msg = "you must selected a platform from the list. You can pass '-h' for a selected platform to list its options."
+ create_project_platforms_parser = create_project_parser.add_subparsers(
+ title="platforms", help=help_msg, dest="platform"
+ )
+ build_platforms_parser = build_parser.add_subparsers(
+ title="platforms", help=help_msg, dest="platform"
+ )
+ flash_platforms_parser = flash_parser.add_subparsers(
+ title="platforms", help=help_msg, dest="platform"
+ )
+
+ # Map used to map a API method to proper CLI parsers and handlers.
+ # API method parser handler smbcmd domain
+ subcmds = {
+ "generate_project": [create_project_platforms_parser, create_project_handler],
+ "build": [build_platforms_parser, build_handler],
+ "flash": [flash_platforms_parser, flash_handler],
+ }
+
+ # Helper to add a platform to a subcmd parser.
+ def _add_parser(parser, platform):
+ platform_name = platform[0].upper() + platform[1:] + " platform"
+ o = parser.add_parser(platform, add_help=False, help=f"select {platform_name}.")
+ o.set_defaults(platform=platform)
+ return o
+
+ parser_by_subcmd = {}
+ for subcmd, o in subcmds.items():
+ parser_by_platform = {}
+ o[0].required = True # Selecting a platform/template is mandatory
+ for platform in TEMPLATES:
+ new_parser = _add_parser(o[0], platform)
+ parser_by_platform[platform] = new_parser
+
+ new_parser = o[0].add_parser("template", add_help=False, help="select an adhoc template.")
+ new_parser.add_argument(
+ "-d", metavar="TEMPLATE_DIR", required=True, help="project API template directory."
+ )
+ new_parser.set_defaults(platform="template")
+ parser_by_platform["template"] = new_parser
+
+ parser_by_subcmd[subcmd] = parser_by_platform
+
+ known_args, unkown_args = main_parser.parse_known_args()
+
+ try:
+ subcmd = known_args.subcommand
+ platform = known_args.platform
+ except AttributeError:
+ # No subcommand or platform, so not need to augment the parser.
+ return
+
+ # Augment parser with project options
+
+ if platform == "template":
+ # adhoc template
+ template_dir = known_args.d
+ else:
+ # default template
+ template_dir = TEMPLATES[platform]
+
+ template = project.TemplateProject.from_directory(template_dir)
+ template_info = template.info()
+
+ options_by_method = get_project_options(template_info)
+
+ # TODO(gromero): refactor to remove this map.
+ subcmd_to_method = {
+ "create-project": "generate_project",
+ "build": "build",
+ "flash": "flash",
+ }
+
+ method = subcmd_to_method[subcmd]
+ parser_by_subcmd_n_platform = parser_by_subcmd[method][platform]
+ _, handler = subcmds[method]
+
+ parser_by_subcmd_n_platform.formatter_class = (
+ argparse.RawTextHelpFormatter
+ ) # Set raw help text so help_text format works
+ parser_by_subcmd_n_platform.set_defaults(
+ subcommand_handler=handler,
+ valid_options=options_by_method[method],
+ template_dir=template_dir,
+ )
+
+ parser_by_subcmd_n_platform.add_argument(
+ "--list-options",
+ action="help",
+ help="show all options/values for selected platforms/template.",
+ )
+
+ required = any([opt["required"] for opt in options_by_method[method]])
+ nargs = "+" if required else "*"
+
+ help_text_by_option = [opt["help_text"] for opt in options_by_method[method]]
+ help_text = "\n\n".join(help_text_by_option) + "\n\n"
+
+ # TODO(gromero): Experiment with required=required below
+ parser_by_subcmd_n_platform.add_argument(
+ "-o", required=True, metavar="OPTION=VALUE", nargs=nargs, help=help_text
Review comment:
Sure, it's better as you suggested. Done in https://github.com/apache/tvm/pull/9229/commits/16635d2e7399cfc03dc58ee0a429c02891d9b239
--
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] gromero commented on pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
gromero commented on pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#issuecomment-961512197
@areusch I've removed the ugly "lookahead" hack and replaced it by a real parser in https://github.com/apache/tvm/pull/9229/commits/409ee6c768517df37300cb6fa529f716e31e7751
The change also now allows using `-h` or `--help` to list the platform options like:
```
gromero@amd:~/git/tvm$ tvmc micro create-project --force /tmp/x46 ~/scripts/sine.tar zephyr --help
usage: tvmc micro create-project project_dir MLF zephyr --project-option OPTION=VALUE [OPTION=VALUE ...] [-h]
optional arguments:
--project-option OPTION=VALUE [OPTION=VALUE ...]
extra_files_tar=EXTRA_FILES_TAR
if given, during generate_project, uncompress the tarball at this path into the project dir.
project_type={aot_demo, host_driven}
type of project to generate. (required)
west_cmd=WEST_CMD
path to the west tool. If given, supersedes both the zephyr_base option and ZEPHYR_BASE environment variable. Defaults to '/usr/bin/python3 -m west'.
zephyr_board={mimxrt1050_evk, mps2_an521, nrf5340dk_nrf5340_cpuapp,
nucleo_f746zg, nucleo_l4r5zi, qemu_cortex_r5, qemu_riscv32,
qemu_riscv64, qemu_x86, stm32f746g_disco}
name of the Zephyr board to build for. (required)
config_main_stack_size=CONFIG_MAIN_STACK_SIZE
sets CONFIG_MAIN_STACK_SIZE for Zephyr board.
-h, --help, --list-options
show this help message with platform-specific options and exit.
```
Which was done in https://github.com/apache/tvm/pull/9229/commits/99f701fd7efc8be34e1e142210ae07f360427898
--
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] areusch commented on a change in pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r746137979
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -38,6 +41,36 @@
class TVMCException(Exception):
"""TVMC Exception"""
+class TVMCSilentArgumentParser(argparse.ArgumentParser):
Review comment:
just curious: was there a reason we couldn't use `add_help=False` for each TVMC ArgumentParser, and then manually add it ourselves at the last minute? this approach requires us to use `_remove_action`, which is marked private.
##########
File path: include/tvm/runtime/device_api.h
##########
@@ -268,6 +268,8 @@ inline const char* DeviceName(int type) {
return "webgpu";
case kDLHexagon:
return "hexagon";
+ case 129:
Review comment:
so this is an RPC device ID. you shouldn't need to define this, and we can't submit this chance to device_api.h (only DeviceType enums should be here
##########
File path: include/tvm/runtime/device_api.h
##########
@@ -268,6 +268,8 @@ inline const char* DeviceName(int type) {
return "webgpu";
case kDLHexagon:
return "hexagon";
+ case 129:
Review comment:
so this is an RPC device ID. you shouldn't need to define this, and we can't submit this change to device_api.h (only DeviceType enums should be here
--
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] areusch commented on a change in pull request #9229: [TVMC][microTVM] Add new micro context
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r748589229
##########
File path: python/tvm/driver/tvmc/common.py
##########
@@ -38,6 +41,36 @@
class TVMCException(Exception):
"""TVMC Exception"""
+class TVMCSilentArgumentParser(argparse.ArgumentParser):
Review comment:
aewsome, that looks great!
--
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] areusch commented on a change in pull request #9229: [TVMC] Add new micro context
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #9229:
URL: https://github.com/apache/tvm/pull/9229#discussion_r737938422
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -384,13 +482,27 @@ def run_module(
else:
logger.debug("Running on remote RPC with no key.")
session = rpc.connect(hostname, port)
+ elif device == "micro":
+ # Remote RPC (running on a micro target)
+ logger.debug("Running on remote RPC (micro target).")
+ try:
+ with ExitStack() as stack:
Review comment:
the ExitStack still only calls `__exit__` when leaving a `with` block (or technically, when _its_ `__exit__` is called). to make this approach work, can you move this line above 475 (so that the entire body of `run_module` past that line is indented inside the with block)? then, calling `enter_session` here will cause `__exit__` to be invoked when the function returns.
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -97,7 +110,57 @@ def add_run_parser(subparsers):
help="hostname (required) and port (optional, defaults to 9090) of the RPC tracker, "
"e.g. '192.168.0.100:9999'",
)
- parser.add_argument("FILE", help="path to the compiled module file")
+ parser.add_argument(
+ "PATH",
+ help="path to the compiled module file or to the project directory (micro devices only).",
+ )
+
+ #
+ # Hack to lookahead if 'run' was selected and make
+ # two dynamic parsers (in micro.py and in runner.py) work.
+ #
+ # print(sys.argv)
Review comment:
nit: rm
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -404,16 +513,24 @@ def run_module(
dev = session.vulkan()
elif device == "rocm":
dev = session.rocm()
+ elif device == "micro":
+ dev = session.device
+ lib = session.get_system_lib()
else:
assert device == "cpu"
dev = session.cpu()
+ # TODO(gromero): Adjust for micro targets.
Review comment:
i think this means that benchmark doesn't work with remote devices. i'd suggest we mark benchmarking as unsupported with micro devices and merge this PR rather than try to make benchmarking support remote devices.
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -97,7 +111,55 @@ def add_run_parser(subparsers):
help="hostname (required) and port (optional, defaults to 9090) of the RPC tracker, "
"e.g. '192.168.0.100:9999'",
)
- parser.add_argument("FILE", help="path to the compiled module file")
+ parser.add_argument(
+ "PATH",
+ help="path to the compiled module file or to the project directory (micro devices only).",
+ )
+
+ #
+ # Hack to lookahead if 'run' was selected and make
+ # two dynamic parsers (in micro.py and in runner.py) work.
+ #
+ # print(sys.argv)
+ if "run" in sys.argv:
Review comment:
er i meant like this on the invert thing.
```
if "run" not in sys.argv:
return
known_args, _ = main_parser.parse_known_args()
```
on the question about detecting "run", not sure i quite follow--let's chat further offline.
##########
File path: python/tvm/micro/project_api/server.py
##########
@@ -42,15 +42,24 @@
_LOG = logging.getLogger(__name__)
-_ProjectOption = collections.namedtuple("ProjectOption", ("name", "choices", "help"))
+_ProjectOption = collections.namedtuple(
+ "ProjectOption", ("name", "choices", "default", "type", "required", "optional", "help")
+)
class ProjectOption(_ProjectOption):
+ """Class used to keep the metadata associated to project options."""
+
def __new__(cls, name, **kw):
"""Override __new__ to force all options except name to be specified as kwargs."""
assert "name" not in kw
+ assert "required" in kw or "optional" in kw, "'required' or 'optional' must be specified."
Review comment:
suggest "at least one of 'required' or 'optional' must be specified."
##########
File path: apps/microtvm/zephyr/template_project/microtvm_api_server.py
##########
@@ -229,40 +229,69 @@ def _get_nrf_device_args(options):
PROJECT_OPTIONS = [
server.ProjectOption(
"extra_files_tar",
+ optional=["generate_project"],
+ type="str",
Review comment:
oh i see. sorry about that, this does make sense then.
##########
File path: python/tvm/driver/tvmc/runner.py
##########
@@ -404,16 +513,24 @@ def run_module(
dev = session.vulkan()
elif device == "rocm":
dev = session.rocm()
+ elif device == "micro":
+ dev = session.device
+ lib = session.get_system_lib()
else:
assert device == "cpu"
dev = session.cpu()
+ # TODO(gromero): Adjust for micro targets.
Review comment:
ah yeah adding to device_api.h isn't going to fix this. the 129 part is a bit confusing--it's actually 1 | (1 << 8) == 1 | 128. 128 is a mask indicating the device is a remote device.
##########
File path: src/runtime/rpc/rpc_module.cc
##########
@@ -164,7 +164,7 @@ class RPCModuleNode final : public ModuleNode {
~RPCModuleNode() {
if (module_handle_ != nullptr) {
try {
- sess_->FreeHandle(module_handle_, kTVMModuleHandle);
+ // sess_->FreeHandle(module_handle_, kTVMModuleHandle);
Review comment:
i see. can you try modifying [TVMModFree](https://github.com/apache/tvm/blob/main/src/runtime/crt/common/crt_runtime_api.c#L186) to be a no-op when `mod == system_lib_handle` (and `system_lib_handle` is not `kTVMModuleHandleUninitialized`)? this may fix your problem. it may also be that we need to reset the board between run calls...but we could try to see if this works first.
##########
File path: python/tvm/micro/project.py
##########
@@ -75,14 +75,23 @@ def __init__(self, api_client, options):
raise TemplateProjectError()
def build(self):
+ assert self._options is not None, "'options' is not set!"
self._api_client.build(self._options)
def flash(self):
+ assert self._options is not None, "'options' is not set!"
self._api_client.flash(self._options)
def transport(self):
+ assert self._options is not None, "'options' is not set!"
return ProjectTransport(self._api_client, self._options)
+ def info(self):
+ return self._info
+
+ def set_options(self, options):
Review comment:
can you use a python [setter](https://python-reference.readthedocs.io/en/latest/docs/property/setter.html) or just make options e.g. `self.options`?
--
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