[TVMC][microTVM] Add new micro context #9229

gromero · 2021-10-08T02:56:38Z

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:

$ 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 --project-option project_type=host_driven zephyr_board=stm32f746g_disco
$ tvmc micro build /tmp/micro_speech zephyr --project-option zephyr_board=stm32f746g_disco
$ tvmc micro flash /tmp/micro_speech zephyr --project-option zephyr_board=stm32f746g_disco
$ tvmc run --device micro /tmp/micro_speech --project-option zephyr_board=stm32f746g_disco --print-top 6 --fill-mode random
[[   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.

areusch

@gromero thanks for posting this up! did a pass and cc @mehrdadh to take a look!

apps/microtvm/zephyr/template_project/microtvm_api_server.py

areusch · 2021-10-12T20:53:24Z

apps/microtvm/zephyr/template_project/microtvm_api_server.py

    server.ProjectOption(
        "west_cmd",
+        optional=["generate_project"],
+        default="python3 -m west",


is this needed? prefer sys.executable if possible

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 efbc280

I also took the chance to add default value to the help message in 9bf6643 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'.

areusch · 2021-10-12T20:53:40Z

apps/microtvm/zephyr/template_project/microtvm_api_server.py

+    server.ProjectOption(
+        "zephyr_base",
+        optional=["build", "open_transport"],
+        default="ZEPHYR_BASE",


why this default?

I think by default we could get it from environment variable ZEPHYR_BASE?

@areusch It was actually wrong. I've fixed as suggested by @mehrdadh . Done in efbc280#diff-e9070632c64e33062450a5c887a7a6b683d7294064584407aeff7aef24031dc1R280

Hence now it shows:

zephyr_base=ZEPHYR_BASE path to the zephyr base directory. Defaults to '/home/gromero/zephyrproject/zephyr'.

looks good.

areusch · 2021-10-12T20:55:52Z

python/tvm/driver/tvmc/common.py

+
+                if opt["type"] == "bool":
+                    # TODO(gromero): Use only choices= and merge with non-bool options below
+                    option_choices_text = f"{name}={{true, false}}"


it might be a bit cleaner to just set choices = ["true", "false"] and reuse the logic below.

Done in caf01be

areusch · 2021-10-12T20:56:16Z

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):


can you add docstring and type annotation if possible?

Sure. Done in 303730a

areusch · 2021-10-12T22:02:07Z

python/tvm/driver/tvmc/runner.py

+    # two dynamic parsers (in micro.py and in runner.py) work.
+    #
+    # print(sys.argv)
+    if "run" in sys.argv:


suggest to invert the condition and return early. also, what if run is e.g. the value of --template-dir?

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.

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.

areusch · 2021-10-12T22:03:59Z

python/tvm/driver/tvmc/runner.py

+    micro = False
+    if device == "micro":
+        if tvmc_package.type != "mlf":
+            TVMCException("--device 'micro' specified but no MLF archive found in PATH.")


i'd phrase it more in terms of which command-line arg is missing

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 8f6f6f6

I also realized that I was not effectively calling raise there so no exception was raised. I fixed that also in the commit above.

areusch · 2021-10-12T22:07:01Z

python/tvm/driver/tvmc/runner.py

    else:
        assert device == "cpu"
        dev = session.cpu()

+    # TODO(gromero): Adjust for micro targets.


can you explain more?

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

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.

For now. I've reverted to use the common code (and so use module.benchmark()): 825376f

And kept the workaround until we decide what to do so the branch is kept working: a63a486

@areusch Do you have any suggestion on how to fix it?

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.

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.

areusch · 2021-10-12T22:23:07Z

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):


seems like open should return nothing and enter should return self

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: 42167f9
Kept the fix for the typo in: bf94efe
Use of contextlib in: 874382e

areusch · 2021-10-12T22:23:17Z

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);


nit: uncomment

This isn't actually a nit :(

Uncommenting it amounts to reverting ce29d5b

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 :)

i see. can you try modifying TVMModFree 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.

mehrdadh

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.

mehrdadh · 2021-10-13T22:02:59Z

apps/microtvm/zephyr/template_project/microtvm_api_server.py

+    server.ProjectOption(
+        "zephyr_base",
+        optional=["build", "open_transport"],
+        default="ZEPHYR_BASE",


I think by default we could get it from environment variable ZEPHYR_BASE?

mehrdadh · 2021-10-13T22:05:56Z

python/tvm/driver/tvmc/common.py

+    return options_by_method
+
+
+def get_options(options):


Could you please add docstring to all methods? it makes it much easier to understand
also maybe add type to arguments?

@mehrdadh Fixed ZEPHYR_BASE as you suggested in efbc280#diff-e9070632c64e33062450a5c887a7a6b683d7294064584407aeff7aef24031dc1R280

@mehrdadh added docstrings for get_options and other functions in

7ed6635
2ff3d80
3771745
7534618

mehrdadh · 2021-10-13T22:12:01Z

python/tvm/driver/tvmc/micro.py

+    )
+    create_project_parser.set_defaults(subcommand_handler=create_project_handler)
+    create_project_parser.add_argument(
+        "PROJECT_DIR",


+1 to lowercase.

mehrdadh · 2021-10-13T22:14:19Z

python/tvm/driver/tvmc/micro.py

+    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


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.

@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.

@gromero you're right. It should be in project API and added automatically to tvmc.

mehrdadh

Added few comments. Please address previous comments.
Also, how do we test tvmc on CI?

mehrdadh · 2021-10-22T18:13:21Z

python/tvm/driver/tvmc/micro.py

+TVM_HOME = os.getenv("TVM_HOME")
+
+
+ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"


you can update these once #9309 is merged.

@mehrdadh Thanks for the heads up. I'll keep and eye on it and take care to rebase once it's merged.

#9309 is merged now.

mehrdadh · 2021-10-22T18:16:49Z

python/tvm/driver/tvmc/runner.py

-            "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


maybe reuse device and get rid of micro variable?

Done in c64d095

gromero · 2021-10-25T17:14:31Z

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

gromero · 2021-10-25T17:17:34Z

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$

gromero · 2021-10-25T17:21:46Z

@areusch @mehrdadh Thanks a lot for the reviews. PTAL.

I think there are only two major issues remaining to get addressed:

mehrdadh · 2021-10-26T01:44:20Z

@gromero I wrote a TVMC test which supports both Zephyr and Arduino when I was testing this PR last week:
TVMC Test
Feel free to reuse it.

apps/microtvm/zephyr/template_project/microtvm_api_server.py

areusch · 2021-10-27T22:59:10Z

python/tvm/driver/tvmc/runner.py

    else:
        assert device == "cpu"
        dev = session.cpu()

+    # TODO(gromero): Adjust for micro targets.


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.

areusch · 2021-10-27T23:00:02Z

python/tvm/driver/tvmc/runner.py

    else:
        assert device == "cpu"
        dev = session.cpu()

+    # TODO(gromero): Adjust for micro targets.


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.

areusch · 2021-10-27T23:00:32Z

python/tvm/micro/project_api/server.py

    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."


suggest "at least one of 'required' or 'optional' must be specified."

areusch · 2021-10-28T00:22:52Z

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);


i see. can you try modifying TVMModFree 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.

areusch · 2021-10-28T00:29:15Z

python/tvm/micro/project.py

+    def info(self):
+        return self._info
+
+    def set_options(self, options):


can you use a python setter or just make options e.g. self.options?

Sure. Done.

areusch · 2021-10-28T00:32:18Z

python/tvm/driver/tvmc/runner.py

+        # Remote RPC (running on a micro target)
+        logger.debug("Running on remote RPC (micro target).")
+        try:
+            with ExitStack() as stack:


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.

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.

areusch · 2021-10-28T00:32:46Z

python/tvm/driver/tvmc/runner.py

+    # Hack to lookahead if 'run' was selected and make
+    # two dynamic parsers (in micro.py and in runner.py) work.
+    #
+    # print(sys.argv)


areusch · 2021-10-28T00:37:05Z

python/tvm/driver/tvmc/runner.py

+    # two dynamic parsers (in micro.py and in runner.py) work.
+    #
+    # print(sys.argv)
+    if "run" in sys.argv:


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.

gromero · 2021-10-28T17:22:23Z

@areusch about modifying TVMModFree to be a no-op when mod == system_lib_handle: it works. Done in e76156d

mehrdadh · 2021-10-28T23:38:44Z

Please add [microtvm] to PR title.

gromero · 2021-11-04T23:39:35Z

@areusch I've removed the ugly "lookahead" hack and replaced it by a real parser in 409ee6c

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 99f701f

include/tvm/runtime/device_api.h

python/tvm/driver/tvmc/common.py

areusch

thanks @gromero! there are a couple of things we should address in a follow-on, but i think we can merge this now.

areusch · 2021-11-12T21:16:12Z

python/tvm/driver/tvmc/runner.py

+    path = pathlib.Path(args.PATH)
+    options = None
+    if args.device == "micro":
+        path = path / "model.tar"


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.

hmr, I see. Yeah I'll prepare a follow-on PR to fix it. 👍

areusch · 2021-11-12T21:17:43Z

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":


can also do this in a follow-on. i think you still need to preserve this check when device != "micro"

yeah, that seems correct. Ok, I'll prepare a follow-on PR for it. 👍

@areusch Actually, it's fixed in this PR, otherwise tests fail ;)

areusch · 2021-11-12T21:21:20Z

@leandron in case you want to look at this too

Make Zephyr platform options comply with RFC-0020 specification. Project options now need to specify the required metadata for every option, i.e. 'required', 'optional', and 'type'. Signed-off-by: Gustavo Romero <[email protected]>

Make Arduino platform options comply with RFC-0020 specification. Project options now need to specify the required metadata for every option, i.e. 'required', 'optional', and 'type'. Signed-off-by: Gustavo Romero <[email protected]>

Make crt project options comply with RFC-0020 specification. Project options now need to specify the required metadata for every option, i.e. 'required', 'optional', and 'type'. Signed-off-by: Gustavo Romero <[email protected]>

Adapt test to new metadata fields accordingly to RFC-0020 specification. Signed-off-by: Gustavo Romero <[email protected]>

Add info() method to GeneratedProject class so one can use the Project API to query options for project dirs instead of only for template projects. This commit also adds for the sake of convenience a setter and a getter for 'options' in case it's necessary to set or get 'options' after a GeneratedProject class is instantiated without initializing 'options'. Signed-off-by: Gustavo Romero <[email protected]>

Fix typo in comment. Signed-off-by: Gustavo Romero <[email protected]>

Currently there is a limitation on microTVM / TVM which doesn't allow running a model multiple times in sequence without previously flashing the model to the device. Root cause is that RPCModuleNode class destructor is called once a run finishes. The destructor sends a RPCCode::kFreeHandle packet with type_code = kTVMModuleHandle to the device which wipes entries in crt/src/runtime/crt/common/crt_runtime_api.c:147:static const TVMModule* registered_modules[TVM_CRT_MAX_REGISTERED_MODULES] when TVMFreeMod() is called when the target receives a kFreeHandle packet. Hence when one tries to re-run a model registered_modules[0] == NULL causes a backtrace on the host side. Probably never before a model on microTVM was run without being flashed just before the run, so tvmc run implementation for micro targets exposed the issue. This commit fixes it by not calling TVMFreeMod() for system_lib_handle on the target side when a session terminates so the pointer to the system_lib_handle is not flushed from 'registered_modules', allowing multiple runs on micro targets. Signed-off-by: Gustavo Romero <[email protected]>

Currently when a add_*_parser functions are called in main.py to build and add the various subparsers to the main parser only a subparser is passed to the functions. However if one of these functions need to build a dynamic parser it needs also to call the main parser at least once to parse once the command line and get the arguments necessary to finally build the complete parser. This commit fixes that limitation by passing also the main parser when calling the subparser builders so it can be used to build the dynamic subparses. Signed-off-by: Gustavo Romero <[email protected]>

This commit introduces support for micro targets (targets supported by microTVM). It creates a new micro context under the new TVMC command 'tvmc micro'. Moreover, three new subcommands are made available in the new context under 'tvmc micro': 'create-project', 'build', and 'flash'. The new support relies on the Project API to query all the options available for a selected platform (like Zephyr and Arduino) and also from any adhoc platform template directory which provides a custom Project API server. Signed-off-by: Gustavo Romero <[email protected]>

Add support for micro devices using the Project API to query all options available for a given platform and open a session with an specified micro device. Use of 'tvmc run' with micro device is enabled via the '--device micro' option in addition to the project directory. Once the project directory is specified 'tvmc run' will make all options specific to the platform found in the project dir available as options in 'tvmc run'. They can be listed by '--list-options' and passed via '--options'. Signed-off-by: Gustavo Romero <[email protected]>

* [microTVM] zephyr: Make platform options comply with RFC-0020 Make Zephyr platform options comply with RFC-0020 specification. Project options now need to specify the required metadata for every option, i.e. 'required', 'optional', and 'type'. Signed-off-by: Gustavo Romero <[email protected]> * [microTVM] arduino: Make platform options comply with RFC-0020 Make Arduino platform options comply with RFC-0020 specification. Project options now need to specify the required metadata for every option, i.e. 'required', 'optional', and 'type'. Signed-off-by: Gustavo Romero <[email protected]> * [microTVM] crt: Make crt options comply with RFC-0020 Make crt project options comply with RFC-0020 specification. Project options now need to specify the required metadata for every option, i.e. 'required', 'optional', and 'type'. Signed-off-by: Gustavo Romero <[email protected]> * [microTVM][Unittest] Adapt test to RFC-0020 Adapt test to new metadata fields accordingly to RFC-0020 specification. Signed-off-by: Gustavo Romero <[email protected]> * [microTVM] Add info() method to GeneratedProject class Add info() method to GeneratedProject class so one can use the Project API to query options for project dirs instead of only for template projects. This commit also adds for the sake of convenience a setter and a getter for 'options' in case it's necessary to set or get 'options' after a GeneratedProject class is instantiated without initializing 'options'. Signed-off-by: Gustavo Romero <[email protected]> * [microTVM] Fix typo in python/tvm/micro/session.py Fix typo in comment. Signed-off-by: Gustavo Romero <[email protected]> * Allow multiple runs on micro targets Currently there is a limitation on microTVM / TVM which doesn't allow running a model multiple times in sequence without previously flashing the model to the device. Root cause is that RPCModuleNode class destructor is called once a run finishes. The destructor sends a RPCCode::kFreeHandle packet with type_code = kTVMModuleHandle to the device which wipes entries in crt/src/runtime/crt/common/crt_runtime_api.c:147:static const TVMModule* registered_modules[TVM_CRT_MAX_REGISTERED_MODULES] when TVMFreeMod() is called when the target receives a kFreeHandle packet. Hence when one tries to re-run a model registered_modules[0] == NULL causes a backtrace on the host side. Probably never before a model on microTVM was run without being flashed just before the run, so tvmc run implementation for micro targets exposed the issue. This commit fixes it by not calling TVMFreeMod() for system_lib_handle on the target side when a session terminates so the pointer to the system_lib_handle is not flushed from 'registered_modules', allowing multiple runs on micro targets. Signed-off-by: Gustavo Romero <[email protected]> * [TVMC] Pass main parser when calling add_*_parser functions Currently when a add_*_parser functions are called in main.py to build and add the various subparsers to the main parser only a subparser is passed to the functions. However if one of these functions need to build a dynamic parser it needs also to call the main parser at least once to parse once the command line and get the arguments necessary to finally build the complete parser. This commit fixes that limitation by passing also the main parser when calling the subparser builders so it can be used to build the dynamic subparses. Signed-off-by: Gustavo Romero <[email protected]> * [TVMC] micro: Add new micro context This commit introduces support for micro targets (targets supported by microTVM). It creates a new micro context under the new TVMC command 'tvmc micro'. Moreover, three new subcommands are made available in the new context under 'tvmc micro': 'create-project', 'build', and 'flash'. The new support relies on the Project API to query all the options available for a selected platform (like Zephyr and Arduino) and also from any adhoc platform template directory which provides a custom Project API server. Signed-off-by: Gustavo Romero <[email protected]> * [TVMC] run: Add support for micro devices Add support for micro devices using the Project API to query all options available for a given platform and open a session with an specified micro device. Use of 'tvmc run' with micro device is enabled via the '--device micro' option in addition to the project directory. Once the project directory is specified 'tvmc run' will make all options specific to the platform found in the project dir available as options in 'tvmc run'. They can be listed by '--list-options' and passed via '--options'. Signed-off-by: Gustavo Romero <[email protected]>

areusch reviewed Oct 12, 2021

View reviewed changes

mehrdadh requested changes Oct 13, 2021

View reviewed changes

mehrdadh requested changes Oct 22, 2021

View reviewed changes

areusch reviewed Oct 28, 2021

View reviewed changes

gromero changed the title ~~[TVMC] Add new micro context~~ [TVMC][microTVM] Add new micro context Oct 29, 2021

gromero closed this Oct 29, 2021

gromero reopened this Oct 29, 2021

areusch reviewed Nov 9, 2021

View reviewed changes

include/tvm/runtime/device_api.h Outdated Show resolved Hide resolved

python/tvm/driver/tvmc/common.py Outdated Show resolved Hide resolved

gromero mentioned this pull request Nov 10, 2021

[RFC][TVMC] Add support for micro targets apache/tvm-rfcs#43

Merged

areusch approved these changes Nov 12, 2021

View reviewed changes

areusch marked this pull request as ready for review November 12, 2021 21:19

areusch requested review from comaniac, icemelon, jroesch, junrushao, jwfromm, leandron, liangfu, manupak, tmoreau89, tqchen, yzhliu and zhiics as code owners November 12, 2021 21:19

gromero force-pushed the tvmc_microtvm branch 7 times, most recently from 2d02edf to 41e7feb Compare November 18, 2021 04:03

gromero requested a review from merrymercy as a code owner November 18, 2021 04:03

gromero force-pushed the tvmc_microtvm branch from 41e7feb to e96722f Compare November 18, 2021 14:50

gromero added 10 commits November 18, 2021 21:08

[microTVM] crt: Make crt options comply with RFC-0020

c0cd0b9

Make crt project options comply with RFC-0020 specification. Project options now need to specify the required metadata for every option, i.e. 'required', 'optional', and 'type'. Signed-off-by: Gustavo Romero <[email protected]>

[microTVM][Unittest] Adapt test to RFC-0020

fcd0e51

Adapt test to new metadata fields accordingly to RFC-0020 specification. Signed-off-by: Gustavo Romero <[email protected]>

[microTVM] Fix typo in python/tvm/micro/session.py

517a134

Fix typo in comment. Signed-off-by: Gustavo Romero <[email protected]>

gromero force-pushed the tvmc_microtvm branch from e96722f to ad45c67 Compare November 18, 2021 21:15

areusch merged commit 53cb5ea into apache:main Nov 19, 2021

This was referenced Dec 1, 2021

TVM forcing compilation with USE_MICRO ON #9617

Closed

[BugFix][Micro] fix tvm.micro ImportError in tvmc when USE_MICRO is OFF #9619

Closed

driazati mentioned this pull request Jul 14, 2022

TVM v0.9.0.rc0 Release Candidate Notes #12102

Closed

		TVM_HOME = os.getenv("TVM_HOME")


		ZEPHYR_TEMPLATE_DIR = TVM_HOME + "/apps/microtvm/zephyr/template_project"

[TVMC][microTVM] Add new micro context #9229

[TVMC][microTVM] Add new micro context #9229

Conversation

gromero commented Oct 8, 2021 • edited Loading

areusch 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

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 Oct 25, 2021 • edited Loading

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 Oct 25, 2021 • edited Loading

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

Choose a reason for hiding this comment

gromero Oct 25, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

gromero Oct 25, 2021 • edited Loading

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

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

gromero commented Oct 25, 2021

gromero commented Oct 25, 2021

gromero commented Oct 25, 2021

mehrdadh commented Oct 26, 2021

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

Choose a reason for hiding this comment

gromero Oct 28, 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 commented Oct 28, 2021

mehrdadh commented Oct 28, 2021

gromero commented Nov 4, 2021

areusch 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

gromero commented Oct 8, 2021 •

edited

Loading

gromero Oct 25, 2021 •

edited

Loading

gromero Oct 25, 2021 •

edited

Loading

gromero Oct 25, 2021 •

edited

Loading

gromero Oct 25, 2021 •

edited

Loading

gromero Oct 28, 2021 •

edited

Loading