[RFC][TVMC] Add support for micro targets #43

gromero · 2021-10-28T22:24:44Z

This RFC is about how TVMC (TVM CLI tool) can be extended to support microTVM
targets, considering the variety of platforms supported by microTVM, like Zephyr
and Arduino, and also considering future platforms, taking into account the use
of custom/adhoc platforms provided by users at their convenience.

This RFC is about how TVMC (TVM CLI tool) can be extended to support microTVM targets, considering the variety of platforms supported by microTVM, like Zephyr and Arduino, and also considering future platforms, taking into account the use of custom/adhoc platforms provided by users at their convenience.

gromero · 2021-10-28T22:26:11Z

cc @areusch @mehrdadh @leandron @guberti

mehrdadh

Thanks for the RFC! LGTM, just small changes.

I have a question about the case when user provides a template directory. This might not necessarily be related to TVMC, but I'm curious to know where do we check if the provided template project API supports required methods such as build, flash, etc.
At the current state of TVMC, do we only see an RPC error if that happens?

mehrdadh · 2021-10-28T23:18:31Z

rfcs/0021-tvmc_add_support_for_micro_targets.md

+
+**[0]** [RFC] TVMC: Add support for µTVM -- https://discuss.tvm.apache.org/t/rfc-tvmc-add-support-for-tvm/9049
+
+**[1]** [RFC][Project API] Extend metadata in ProjectOption -- https://github.com/apache/tvm-rfcs/pull/33


maybe refer to the merged RFC now?
https://github.com/apache/tvm-rfcs/blob/main/rfcs/0020-project_api_extend_metadata.md

mehrdadh · 2021-10-28T23:21:19Z

rfcs/0021-tvmc_add_support_for_micro_targets.md

+selected as an option under 'build' will become available.
+
+`flash` can be used for effectively flashing, to the selected board, the
+firmware built. Again, pertinent options for 'flash' context considering the


change to built firmware?

argh, sure, thanks! Fixed.

mehrdadh · 2021-10-28T23:22:40Z

rfcs/0021-tvmc_add_support_for_micro_targets.md

+
+A **key-value pair** is used to specify the options specific to the platforms.
+If the option is of type 'bool' the values available are 'true' and 'false'.
+Details on project option types are described in **RFC-0020**.


add link for RFC-0020?

gromero · 2021-10-29T20:03:29Z

@mehrdadh thanks for the review!

Yes, if a Project API method implementation is absent a TypeError will be raised at the server side because the abstract method is not implemented when the Handler class is derived. Also as per https://github.com/apache/tvm-rfcs/blame/main/rfcs/0008-microtvm-project-api.md#L363 I understand the implementation for all of the methods are indeed mandatory.

Thus, just as a reference something similar to the following will happen:

gromero@amd:~/git/tvm$ tvmc micro create-project --force /tmp/x46 ~/scripts/sine.tar zephyr --project-option zephyr_board=stm32f746g_disco project_type=host_driven 
Traceback (most recent call last):
  File "/home/gromero/git/tvm/apps/microtvm/zephyr/template_project/microtvm_api_server.py", line 739, in <module>
    server.main(Handler())
TypeError: Can't instantiate abstract class Handler with abstract method build
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 110, in from_directory
    return cls(client.instantiate_from_dir(template_project_dir))
  File "/home/gromero/git/tvm/python/tvm/micro/project.py", line 114, 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 119, in _request_reply
    raise ConnectionShutdownError("got EOF reading reply from API server")
tvm.micro.project_api.client.ConnectionShutdownError: got EOF reading reply from API server

mehrdadh

@gromero thanks for evaluating that case. LGTM!

areusch

@gromero sorry for the delay here. this looks great! a couple small questions here i'd just like to resolve before accepting/merging.

cc @leandron PTAL if you'd like to provide some feedback before we proceed here.

areusch · 2021-11-08T17:37:11Z

rfcs/0021-tvmc_add_support_for_micro_targets.md

+
+$ tvmc micro flash /tmp/x200 zephyr
+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


just curious: why is -o a required arg? seems it should be always optional (i agree zephyr_board is required for the zephyr microtvm_api_server.py, but as an API-level concept, it shouldn't always be required to specify -o).

@areusch that's right it should not be always required. It was as a TODO in the Draft PR. I think that if one or more required options exist in the project then -o must be required, otherwise that should be optional. (actually now --project-options - I'll update the text accordingly to the reviews in the code that changed the name of that option). It's now done in apache/tvm@93bb565

As a reference, if all options in zephyr are optional, after that change tvmc will show --project-option between [ ] and also no option in the help text appears marked with (required):

$ 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 ...]] [-h] optional arguments: --project-option [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. 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. 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.

areusch · 2021-11-08T17:38:40Z

rfcs/0021-tvmc_add_support_for_micro_targets.md

+[...]
+```
+
+Once both `micro` device and project directory are specified `--list-options`


would --help not do this? it's okay to go with this route, just curious

I think that --list-options is ok, given that it makes clear to an end user that those options are dynamic options, implemented by the underlying template.

@areusch in essence, and specifically to tvmc run command, this is related to the issue of parse_known_args() "intercepting" --help (and -h) because add_help = True in the parsers everywhere. Hence that's related to your comment in apache/tvm#9229 (comment).

I was able to resolve it for tvmc micro tho, so --help and --list-options are aliases and show the dynamic options. Thus if we could keep --list-options in the RFC it would work for both run and micro subcommands when listing the options. But on the other hand I'll experiment as you suggested anyway (setting add_help=False on every parser)

leandron

LGTM, I'm very happy with the results we got with this.

leandron · 2021-11-09T17:27:36Z

rfcs/0021-tvmc_add_support_for_micro_targets.md

+` _ffi.libinfo.find_lib_path()` and so can correctly find the libraries taking
+into account if TVM being used is from sources or from a installed package.
+
+Hence for the specific case of the template directories a new function named


Do templates have a friendly name in the metadata or they are presented as the names of the directories such as "arduino" and "zephyr"? This is just out of curiosity, no need to add or change anything.

at the moment we are using zephyr/arduino and crt( for standalone host crt).

gromero · 2021-11-10T02:36:32Z

@leandron Thanks a lot for the review.

areusch · 2021-11-17T22:22:01Z

thanks @leandron @mehrdadh @gromero, the RFC is accepted! please open a tracking issue for this RFC.

mehrdadh requested changes Oct 28, 2021

View reviewed changes

review mehrdadh: Update RFC-0020 links, fix grammar

03f7d6b

mehrdadh approved these changes Nov 1, 2021

View reviewed changes

areusch reviewed Nov 8, 2021

View reviewed changes

leandron approved these changes Nov 9, 2021

View reviewed changes

areusch approved these changes Nov 17, 2021

View reviewed changes

areusch merged commit 2c345b0 into apache:main Nov 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC][TVMC] Add support for micro targets #43

[RFC][TVMC] Add support for micro targets #43

gromero commented Oct 28, 2021

gromero commented Oct 28, 2021

mehrdadh left a comment

mehrdadh Oct 28, 2021

gromero Oct 29, 2021

mehrdadh Oct 28, 2021

gromero Oct 29, 2021

mehrdadh Oct 28, 2021

gromero Oct 29, 2021

gromero commented Oct 29, 2021

mehrdadh left a comment

areusch left a comment

areusch Nov 8, 2021

gromero Nov 10, 2021 •

edited

Loading

areusch Nov 8, 2021

leandron Nov 9, 2021

gromero Nov 10, 2021 •

edited

Loading

leandron left a comment

leandron Nov 9, 2021

mehrdadh Nov 9, 2021

gromero commented Nov 10, 2021

areusch commented Nov 17, 2021


		[0] [RFC] TVMC: Add support for µTVM -- https://discuss.tvm.apache.org/t/rfc-tvmc-add-support-for-tvm/9049

		[1] [RFC][Project API] Extend metadata in ProjectOption -- https://github.com/apache/tvm-rfcs/pull/33

[RFC][TVMC] Add support for micro targets #43

[RFC][TVMC] Add support for micro targets #43

Conversation

gromero commented Oct 28, 2021

gromero commented Oct 28, 2021

mehrdadh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gromero commented Oct 29, 2021

mehrdadh left a comment

Choose a reason for hiding this comment

areusch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gromero Nov 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gromero Nov 10, 2021 • edited Loading

Choose a reason for hiding this comment

leandron left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gromero commented Nov 10, 2021

areusch commented Nov 17, 2021

gromero Nov 10, 2021 •

edited

Loading

gromero Nov 10, 2021 •

edited

Loading