Enable ability to override list values in config via CLI #1430
+19
−5
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Problem
Previously, we could not override config values that were within a list field via the CLI, ex.
oumi evaluate -c configs/recipes/smollm/evaluation/135m/eval.yaml --tasks[0].num_samples=1
, getting the erroromegaconf.errors.ConfigTypeError: Cannot merge DictConfig with ListConfig
. This is not a problem with using the wrong CLI syntax, but instead with how we were parsing the CLI overrides.Currently, we create a config from the args list with
OmegaConf.from_cli(arg_list)
, and then merge it with the YAML config viaOmegaConf.merge(*all_configs)
. The YAML config is an OmegaConf structured config, meaning it's aware of the typing of our Config object,EvaluationConfig
in this example. The problem is that the config from args list is created before it's merged with the YAML config, so it is not aware of the typing. In the above example with--tasks[0].num_samples=1
, it's thus not aware thattasks
is a List, and creates a dict instead:{"tasks": {"0": {"num_samples": 1}}}
Solution
The solution is simply to call
config.merge_with_dotlist(arg_list)
instead, which merges each argument into the config individually. With this method, the0
is properly interpreted as a list index instead of a dict key.I've tested that this does not impact existing behavior. I've also verified that many variants of specifying overrides, which are Pythonic and we expect to work, do work. Here's some example args for the config mentioned in the first example:
NOTE: This PR doesn't support the ability to add new elements in the list. This is currently not supported by OmegaConf: facebookresearch/hydra#1547 (comment)
Related issues
Fixes OPE-934
Before submitting