cc @leandron @areusch @manupa-arm @tgall_foo @Mousius @mehrdadh
Thanks @gromero for the RFC. Overall looks good to me. I have few comments/suggestions:
- considering the flow of using tvmc for micro, I find it’s a little bit confusing to switch between
tvmcandtvmc microback and forth. My suggestion is, if possible, to have all tasks for microTVM tvmc to live undertvmc microincluding [tvmc micro compile,tvmc micro run, etc] and then reusetvmcinterface in the backend. This will also resolve the drawback that you mentioned, because you can limitargumentsfortvmc micro runand pass it totvmc run.
I think this is a great idea for next version. I suggest that we keep compile option, but change create-project to compile and create the project.
Another suggestion that I have is renaming create-project to create. project in this context doesn’t really differentiate this subcommand to others.
@mehrdadh Thanks a lot for the review!
- considering the flow of using tvmc for micro, I find it’s a little bit confusing to switch between
tvmcandtvmc microback and forth. My suggestion is, if possible, to have all tasks for microTVM tvmc to live undertvmc microincluding [tvmc micro compile,tvmc micro run, etc] and then reusetvmcinterface in the backend. This will also resolve the drawback that you mentioned, because you can limitargumentsfortvmc micro runand pass it totvmc run.
As a general “rule” I’ve tried to stick with not duplicating existing commands when adding the new micro context.
Regarding the compile command, besides avoiding duplicated commands the reasons to separate it from micro build and not creating a compile command inside the “micro” context are also along the lines of what was previously discussed with @leandron , @manupa-arm , and @areusch in [0] – comment #7 I think it’s being working fine that split so far.
Regarding the run command, although creating a micro run would avoid the drawback mentioned it would also duplicate all the options that are common to the non-micro targets inside micro run. Also the more we go this way the more tvmc micro will look like “a tvmc (micro) inside tvmc”, like an whole new tool for micro targets only. So there are caveats with that approach too. But I’m happy to debate more about it.
I think this is a great idea for next version. I suggest that we keep
compileoption, but changecreate-projectto compile and create the project.
If the compile command you mention here is the one that already exists (i.e. tvmc compile) I agreed! tvmc micro create-project can perfectly accommodate your suggestion - compile and create the project, yep.
1 Like
Sure. I have no strong feelings about it. If nobody objects, I’m happy to change it in v2. Thanks.
1 Like
@gromero thanks for writing this up! broadly looks good, a couple points to ask about:
In that sense, the following subcommands are available under
tvmc micro:1. create-project
2. build
3. flash
it might be worth just stating here that build and flash are intended to help with debugging inference flows built on top of this infrastructure. they’re not intended to replace the user’s workflow (e.g. users can still build and flash independently of this).
The main reason for using that “dynamic parser” mechanism is due to the fact that querying the options for a platform takes about 0.4 seconds.
I’d also say that it’s possible for the user to supply their own platform, so it’s not possible to enumerate all platforms’ commands in advance.
This maybe can be solved by enforcing the mutual exclusive groups in the code instead of in the parser as it’s done in the current implementation and mark these options with something like
(not available for micro targets), just as required options are marked with(required).
seems reasonable.
could you explain this one a bit more?
i suggest we keep create-project in case we ever need to create something else. we may even consider e.g. generate-project instead. however, it is a bit wordy if we do expect people to type this a lot…in this case if we decide we should shorten it, i’d propose we keep the longer version and introduce an alias.
1 Like
Fair enough. I’ll put that reason first.
Sure. I meant that currently one does:
$ 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"`
and then:
tvmc micro create-project /tmp/micro_speech ./micro_speech.tar zephyr -o project_type=host_driven zephyr_board=stm32f746g_disco
But since in 2. the board is given (zephyr_board=stm32f746g_disco) a default value for that board can be used for options --target, --pass-config, and --disabled-pass in 1. so tvmc micro create-project can be enhanced to also take care of compiling the model, so step 1. can be skipped. Of course, tvmc compile will always be available if one would like to take full control of the these options when compiling a model.
I chose “create” instead of “generate” indeed because I thought of “generate” being wordy. But honestly I think “create” is just a little better in that sense. I’ll keep create-project for now. Of course I can use an alias cp for it – or create. @mehrdadh If you were also concerned about the boredom of typing -project adding the alias might also help partially – if there are no objections.
As a reference it would look something like:
gromero@amd:~/git/tvm/python/tvm/driver/tvmc$ tvmc micro -h
usage: tvmc micro [-h] {create-project,create,cp,build,flash} ...
optional arguments:
-h, --help show this help message and exit
subcommands:
{create-project,create,cp,build,flash}
create-project (create, cp)
create a project template of a given type or given a template dir.
build build a project dir, generally creating an image to be flashed, e.g. zephyr.elf.
flash flash the built image on a given micro target.
1 Like
I like the idea of reusing previous arguments and I think JSON file could work.
1 Like
I think using alias is perfect, but this looks weird:
Blockquote subcommands: {create-project,create,cp,build,flash}
is there a way to make it like this:
gromero@amd:~/git/tvm/python/tvm/driver/tvmc$ tvmc micro -h
usage: tvmc micro [-h] {create-project,build,flash} ...
optional arguments:
-h, --help show this help message and exit
subcommands:
{create-project,build,flash}
create-project (create, cp)
create a project template of a given type or given a template dir.
build build a project dir, generally creating an image to be flashed, e.g. zephyr.elf.
flash flash the built image on a given micro target.
I thought that too at first, but honestly on a second thought that’s perfectly correct. Think about (create, cp) text there but without that information in the list of subcommands, to me it would be a bit tricky to find out that it means that it is an alias for create-project subcommand. Either way, to the best of my knowledge, there is not way to change it.
ah i understand. yeah this makes sense to me. it might help to clarify that by “departs from a compiled model” you mean “may supply a model in one of the frameworks’ format (e.g. TFLite, ONNX, etc), and therefore may need to also supply a target string.” But also, you do have this problem when running compile as well, just there is no way to lookup the target string from Project API. i’m not suggesting we necessarily need to add that (can always do that in a v2), but just saying it’s merely that you’re including one step inside another.
i think the formatting is ok, but we should choose something else than cp since cp means copy to many linux folks.
1 Like
By “compiled model” in the text I meant the one already compiled and kept in a MLF archive… did you mean “non-compiled model”? 
Either way I agree it’s better to cite the input formats like TFLite, ONNX, etc to avoid confusion when I say "[...] could depart from a **model not compiled** and deduce a default working set of options [...]" 
I’ll change it. Thanks.
yeah I was actually thinking of that as a duty for tvmc only. It would be tvmc that would keep a table associating boards vs default target options, not necessarily for all boards.
Yep, I was thinking of it more for a v2.
@areusch @mehrdadh Thanks for the initial reviews and inputs. I’ve fixed some grammar in the text I found on the second read and incorporated your suggestions.
I believe I can go ahead promoting that to a RFC by creating a PR at https://github.com/apache/tvm-rfcs/ ?
1 Like
Thanks for the changes. LGTM!
1 Like