Fix the overwriting of nested parameters with OmegaConfigLoader

[noklam]

Introduction

A user point out that overring nested parameters was not documented. It was not a well known feature and it was only possible via CLI in early 0.18 series.

Add an example of how to do this via CLI or should use the runtime_params resolver as user pointed out.

Hi team, a question when using Kedro params at runtime:
Right now i have a project with the following parameters.yml (example):
aaa:
bbb:
aba: "2023-11-01"
abb: "2023-11-01"
abc: 14
I was using Kedro 0.18.2, and running kedro command with the following works fine:
kedro run --pipeline deploy --params="aaa.bbb.aba=2023-12-01, aaa.bbb.abb=2023-12-01"
Recently we are deploying a new image which got the version bumped to 0.18.14, and now the pipelines throws the following error:
KeyError: 'abc'
which implies aaa.bbb.abc in this nested parameter is not presented anymore. May I know is it an expected change? We read the docs but it does not specify nested parameter overwriting rules at runtime (https://docs.kedro.org/en/latest/configuration/parameters.html#how-to-specify-parameters-at-runtime)
and what would the suggested solution for such case? Thanks in advance 🙏

[noklam]

[astrojuanlu]

https://linen-slack.kedro.org/t/16184322/hi-team-a-question-when-using-kedro-params-at-runtime-right-#ad3c8635-2a81-45be-8242-349287387628

[noklam]

A more updated fork seems to suggest this is a bug. https://github.com/chy111126/debug_kedro_0182

The problem is the use of env and CLI --params is interacting weirdly.

1. kedro run --env=add --params="aaa.bbb.abb=2023-11-11"

{'aaa': {'bbb': {'abb': '2023-11-11'}}, 'xyz': {'asdf': 123123}, 'def': {'gg': 123}}

2. kedro run --params="aaa.bbb.abb=2023-11-11"

{'aaa': {'bbb': {'aba': '2023-11-01', 'abb': '2023-11-11', 'abc': 14}}, 'xyz': {'asdf': 123123}}

The add env only contains one top key def parameter which is not a conflicted key, but the result override the top level key aaa.

[noklam]

Further testing, I try to switch to ConfigLoader with 0.18.14 and the issue is gone. I did another test to upgrade to 0.19.1 with OmegaConfig and the issue is back.

[noklam]

A temporary workaround is to use settings.py, note this shouldn't be needed because the top-level key has no conflict. We should still investigate the issue.

CONFIG_LOADER_ARGS = {
    "merge_strategy": {"parameters": "destructive"},
}

[noklam]

config={'aaa': {'bbb': {'aba': '2023-11-01', 'abb': '2023-11-11', 'abc': 14}}, 'xyz': {'asdf': 123123}}
env_config={'def': {'gg': 123}, 'aaa': {'bbb': {'abb': '2023-11-11'}}}
resulting_config={'aaa': {'bbb': {'abb': '2023-11-11'}}, 'xyz': {'asdf': 123123}, 'def': {'gg': 123}}

It appears that the --params="aaa.bbb.abb=2023-11-11" somehow get pass into to the add environment as parameters, thus the destructive merge overriding it.

[noklam]

In a discussion today, we found out that #3290 changes the simple dict merge to a OmegaConf.merge, and is likely the root cause of a few bugs.

@SajidAlamQB Do you remember why the changes to context.params necessary?
(p.s. I just revert the changes and test and the problem persist, probably something else happening)

[SajidAlamQB]

@SajidAlamQB Do you remember why the changes to context.params necessary? (p.s. I just revert the changes and test and the problem persist, probably something else happening)

At the time Kedro supported a custom syntax for --params but the decision was made in, #1965, to use standard syntax by using OmegaConf only. I believe we wanted Kedro to more closely use OmegaConf for managing configurations to make things simpler.

[noklam]

@SajidAlamQB make sense, it's still unclear whether OmegaConf.merge affect this issue, it is more clear that it causes #3620. Omegaconf expect raw config, but the config would have resolved at the point. It causing issue when the configuration has non-primitive type

Was the main benefit getting rid of the custom nested dictionary update?

[SajidAlamQB]

Was the main benefit getting rid of the custom nested dictionary update?

I guess the main benefit would be to fully embrace OmegaConf. By removing custom syntax code that we need to maintain for nested dictionaries, we can simplify it by just using OmegaConf's features. But I see its introduced some problems, I still feel its worth trying to get this to work.