Closed PhilippvK closed 2 years ago
Thanks for putting some thought into this!
Your proposal looks good to me.
Your idea to solve limitation 1 seems reasonable.
We could use the possibility to pass multiple arguments for --configs
to mitigate limitation 2 by treating each arguments as seperate multiplicative combination. In the example
--features _ muriscvnn cmsisnn --configs vext.vlen=64 vext.vlen=128 vext.vlen=256 --configs _ tvmaot.target_device=arm_cpu
we would get 3x3x2 runs. Incompatible combinations should only produce an error on the runs, not the whole command. I was not yet aware of specifying multiple --targets
. Does that also work with backends? How is it handled so far if some config/feature is not available for some backends/targets? Should we then for consistency also allow --target spike ovpsim
?
The naming should probably be more explicit. How about --feature_combinations
or --add_feature_combinations
. Or we go another way by treating everything as combinations by default (like we already do with backend/target?) and add parameters for always-applied options - maybe --fixed_config
.
@rafzi Thank you for your feedback!
I agree that some consistency would be nice.
We could use the possibility to pass multiple arguments for --configs to mitigate limitation 2 by treating each arguments as seperate multiplicative combination. In the example
Good idea!
Does that also work with backends?
As there are no incompatible combinations of targets and backends you can specify as many --target
and --backend
flags a you want in a single command.
Should we then for consistency also allow --target spike ovpsim?
This would be an option. However argparse might fail to parse sometimes with the arguments a bad way given that the number of arguments required by those flags is not fixed. See this example:
mlonmcu flow run sine_model --backend tflmi tvmaot # works
mlonmcu flow run --backend tflmi tvmaot -v sine_model # works
mlonmcu flow run --backend tflmi tvmaot sine_model # does not work. Is sine_model a backend or a model?
mlonmcu flow run --backend tflmi tvmaot - sine_model # Solution for the above issue
I agree that this issue exists anyway however as more we rely on those nargs="+"
arguments the more likely it gets to run into this kind of stuff. As shown above there are proper workarounds for this (separating the models with and -
or further arguments) so if we ant to support this, that should be possible.
The naming should probably be more explicit. How about --feature_combinations or --add_feature_combinations. Or we go another way by treating everything as combinations by default (like we already do with backend/target?) and add parameters for always-applied options - maybe --fixed_config.
I think we can agree on a good naming. I am open for those proposals but would prefer if the shorthand flag for the global configs would still be (-c
) which is way more comfortable to type.
Incompatible combinations should only produce an error on the runs, not the whole command
Error occurring during the run will result in a Failing
column being added to the report. If there was at least one failing run in the report, the return code of mlonmcu flow run
is going to be non-zero. We could change this behavior if required. The initialization of features happens before the processing of the run an thus result in exceptions/assertions which might not be catched properly. So maybe we need to distinguish between incompatible and failing runs in the end.
How is it handled so far if some config/feature is not available for some backends/targets?
Configs which do not apply to a given component (feature/target/backend/...) are just ignored.
For now i will just go with the flags --feature-gen
and --config-gen
.
Will merge into main later
A current limitation of the MLonMCU command line interface
mlonmcu flow run
is that all the supplied features and configs are applied globally to all created runs. Such an issue does not exist for the Python API.The workaround for the described problem would be to either use the Python API or run multiple
mlonmcu flow run
commands after each other. The latter approach has the disadvantage that the runs can not be parallelized which may be relevant depending on the number of specified runs.Let's discuss if we should get rid of this limitation. Here is my proposal:
--config foo=bar --config a=b
(A) or a single--config foo=bar a=b
(B).--configs a=b c=d ...
which behaves like (B) but each one of its arguments creates a new run.--config
flag can than be used as usual to pass global configs while the--configs
generates combinations of runs--feature
flag by introducing a--features
flag which accepts several arguments.Example usage
Limitations
--features a,b c
which would be very messytflmi
backend while passing a TVM-only feature using the--feature(s)
flag. An approach to solve this would be just skipping the creation of runs which would not be supported instead of failing.