Open PhilippvK opened 2 years ago
Another problem we have in the current implementation is the following:
We can currently not guarantee the types a config option, aka we have to explicitly cast everything using a @property
to access the dict. However, special cases such as None
should be handled differently which would increase the complexity of such "getter" functions.
In addition I found the issue that we currently can not detect false
via the command line option because bool("false")
does return true
. Instead we would need bool(distutils.util.strtobool(some_string))
which should not be user-facing and indeally happen automatically. An alternative would be a similar approach to the configuration passed via the environments.yml
. Here, booleans seem to be detected automatically, but I have to double check that. Eventually using ast.literal_eval(x)
would help as well.
An detailed overview of the existing configuration mechanics in MLonMCU can be found here: [Will be added later]
TL;DR
Every relevant entity (frontend, framework, backend, target) has a name as well as a
self.config
dictionary as well as class variablesDEFAULTS
andREQUIRED
. The entity’s name will be used as a prefix on the global config layer for mapping configs to specific instances and overwrites the defined defaults (tflmi.arena_size
->tflmi.config["arena_size"]
). TheREQUIRED
variable contains a list of all config keys which need to be defined explicitly beforehand. The mapping of that configuration as well as validating the keys is done in the constructors right now.The mentioned scheme has some disadvantages I we should get rid off like i.e. • a lot of redundant code because every entity handles its configuration separately • No possibility to update the configuration after the constructor without potentially breaking something • No easy way to quickly export all configuration into a single dict. • Class variables are used to define default values which is a good practice (the reason for this was to get the required config keys of a class without instantiating it first.
My proposal for a configuration refactoring is as follows: • Implement a
Config
class which acts as a “smart” replacement for theself.config
dicts, also handling defaults, required keys and the mapping of prefixed keys. • Optional: Add subclasses for i.e. BackendConfig, TargetConfig,… (only if necessary) • Optional: Add abstractConfigurable
base class to all relevant classes (required?) • On the run-level a single config should be shared between all relevant objects (no copying! if a frontend setsconfig["tflmi.arena_size"] = 1024
, it will instantly be available in thetflmi
backend) • This also allows an easy export of all config to a report as we do not need to collect every single config manually as everything is stored a a common place. In addition it would be possible to only export config values which not not match their default value.