Command Line Composition from Internal Registry

[Mousius]

No description provided.

[leandron]

I think this will greatly improve the usability and way we can offer documentation for tvmc users, and make it less scattered removing the need for multiple things encoded in the target string.

It would be to wait a bit for opinions from others, cc @areusch @gromero @comaniac

[comaniac]

So the TVMC argument becomes like the following?

tvmc --target=xx,c
--target-xx-attr
--target-cpu-mcpu
--target-cpu-device
--target-cpu-mtriple
--target-cpu-mattr
--target-cpu-mfloat-abi

Seems a bit tedious. How about the following:

tvmc --target=xx,c
--target-xx "-attr=..."
--target-cpu "-mcpu=x, -device=x, -mtriple=x, -mattr=x, -mfloat-abi=x"

We can still leverage the same idea as list configs to show the detail attributes of each target in this approach. You just need to embed the extracted information to the argparse description.

[Mousius]

There's two main reasons for using the more verbose form:

1. There's no need for a user to learn a sub-command syntax as they do now, they just read the help and add arguments as they need. Typing typically takes less time than learning a new syntax and remembering to switch between them. If we maintain them embedded in a bespoke string I don't think we gain much over the current Target string in terms of understandability?
2. Ideally, in combination with Command Line Configuration Files if you have a complex set of arguments you should be saving it in a configuration file and using it to populate defaults (or in future you may be able to do this as environment variables as @hogepodge suggested 😸 ):

tvmc --config=./dev_board.json --target-c-mcpu=cortex-m55

[comaniac]

I agree that the first point is more user friendly; while my thinking was more developer friendly. I would not against this RFC if others feel it's fine to have verbose form in the CLI. One suggestion might be making this verbose form hierarchical, such as sub-commands of --target; otherwise it may scare people when they see tons of "helps" popping out with just tvmc -h.

[Mousius]

I've added groupings to the example, I think you're right if they're all just blasted onto the screen it's a bit scary but in groups it's similar to other CLIs where you just scroll to the section you're interested in. We can also iterate on help as we go if we think it's getting overwhelming.

Re: user vs developer, I've always assumed tvmc is a user facing tool so I started heading in that direction, @leandron is probably the right person to make the call though.

[leandron]

I agree that the first point is more user friendly; while my thinking was more developer friendly. I would not against this RFC if others feel it's fine to have verbose form in the CLI. One suggestion might be making this verbose form hierarchical, such as sub-commands of --target; otherwise it may scare people when they see tons of "helps" popping out with just tvmc -h.

The thinking here is that the targets encode a lot of information which is hard to explain and expose to both users (not interested in TVM internals) and developers (interested in TVM internals). This will make it more self-explanatory for each argument that can be user from each target.

At the moment, you need a lot of internal knowledge about TVM source codes to understand which options are accepted for the targets.

In that sense, I think other tools like clang are a good example, in which despite the number of arguments potentially becoming large (as you raise with the output of tvmc -h), at least the existing arguments are structured and visible.

So IMHO this change is a very important step in making all interfaces that deal with target setting more transparent.

[areusch]

thanks @Mousius for raising this! a couple questions for you

[Mousius]

Thanks for the feedback @areusch, I've replied to each of your comments except the last one around heterogeneous targets. Please take another look 😸

[Mousius]

Hi @areusch, I think I've addressed the changes you requested, could you take another look?

[areusch]

@Mousius okay sorry i found a code point to the area i was concerned about. good to merge/continue with this after we resolve that point. and cc @junrushao1994 @zxybazh who did some work on the target JSON format.