Experimental omegaconf+ parser mode that supports interpolation across configs and arguments

[mauvilsa]

What does this PR do?

Fixes #439
Fixes #461
Fixes Lightning-AI/pytorch-lightning#13607
Fixes Lightning-AI/pytorch-lightning#15109
Fixes Lightning-AI/pytorch-lightning#20722

Before submitting

• Did you read the contributing guideline?
• Did you update the documentation? (readme and public docstrings)
• Did you write unit tests such that there is 100% coverage on related code? (required for bug fixes and new features)
• Did you verify that new and existing tests pass locally?
• Did you make sure that all changes preserve backward compatibility?
• Did you update the CHANGELOG including a pull request link? (not for typos, docs, test updates, or minor internal changes/refactors)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (9fce2b9) to head (a74a43c).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #765   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        22           
  Lines         6933      6966   +33     
=========================================
+ Hits          6933      6966   +33     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[carmocca]

Thanks for this! It's a great feature.

I tried with my codebase and one big issue is that the "key path" is now absolute instead of local to the file.

For instance, before this worked

python my_jsonargparse_cli.py --job_config=job_config.yaml

And job_config.yaml example:

foo:
  f: ${bar.b}
bar:
  b: 123

But with this change, it requires the following

foo:
  f: ${job_config.bar.b}
bar:
  b: 123

Is this expected? This job config key is under a nested key like so

parser = new_parser(add_config=True)
parser.add_function_arguments(launch, skip={"filepath", "job_config"})
parser.add_class_arguments(JobConfigDataclass, nested_key="job_config", instantiate=False)

Not being backwards compatible is a problem for seamlessly upgraing to omegaconf+

[carmocca]

Also, following my own recipe as described in #479 (comment). The following breaks with omegaconf+:

class Foo:
    def __init__(self, f=2):
        ...

class Bar:
    def __init__(self, b=3):
        ...

def fn(foo: Foo = Foo(), bar: Bar = Bar()):
    ...


from jsonargparse import ArgumentParser, ActionConfigFile
from jsonargparse import get_loader, set_loader

def enable_backwards_compatibility(parser: ArgumentParser) -> None:
    loader = get_loader(mode=parser.parser_mode)

    def custom(config_input: str):
        return loader(config_input)

    set_loader("backwards_compatibility_loader", custom)
    parser.parser_mode = "backwards_compatibility_loader"

parser = ArgumentParser(parser_mode="omegaconf+")  # <- change this
enable_backwards_compatibility(parser)

parser.add_argument("-c", "--config", action=ActionConfigFile)
parser.add_function_arguments(fn)
args = parser.parse_args()
print(args)

foo:
  class_path: __main__.Foo
  init_args:
    f: ${bar.init_args.b}
bar:
  class_path: __main__.Bar
  init_args:
    b: -3

python repro.py --config=config.yaml

Before it would print

Namespace(config=[Path_fr(config.yaml, cwd=/home/ubuntu/carlos/titan)], foo=Namespace(class_path='__main__.Foo', init_args=Namespace(f=-3)), bar=Namespace(class_path='__main__.Bar', init_args=Namespace(b=-3)))

But with omegaconf+ it doesnt properly resolve the argument

Namespace(config=[Path_fr(config.yaml, cwd=/home/ubuntu/carlos/titan)], foo=Namespace(class_path='__main__.Foo', init_args=Namespace(f='${bar.init_args.b}')), bar=Namespace(class_path='__main__.Bar', init_args=Namespace(b=-3)))

[mauvilsa]

@carmocca thanks for the feedback! I will take a look at it. For now

I tried with my codebase and one big issue is that the "key path" is now absolute instead of local to the file.
Is this expected?

Yes, it is expected as mentioned in experimental-omegaconf-mode. That is why I implemented it as a separate omegaconf+, since it is a breaking change.

This feature had been around for long and this is how I finally found a way to implement it. And the key paths didn't seem to be a big issue because it is trivial to do relative. So you would need to change from ${bar.b} to ${.bar.b}.

[carmocca]

Makes sense. Backwards compatibility of configs is a big deal to me so without a mechanism to "upgrade" existing input configs to use the new format ("." prefix) then I can't make this switch to omegaconf+.

[mauvilsa]

Also, following my own recipe as described in #479 (comment). The following breaks with omegaconf+:

omegaconf+ does resolving at the end of parsing, not at load time. If you change the parser mode away from omegaconf+ then there will be no interpolation.

Could be instead

def enable_backwards_compatibility(parser: ArgumentParser) -> None:
    loader = get_loader(mode=parser.parser_mode)

    def custom(config_input: str):
        return loader(config_input)

    set_loader(parser.parser_mode, custom)

[mauvilsa]

I tried with my codebase and one big issue is that the "key path" is now absolute instead of local to the file.
Is this expected?

Yes, it is expected as mentioned in experimental-omegaconf-mode. That is why I implemented it as a separate omegaconf+, since it is a breaking change.

@carmocca with #774 you can now optionally enable conversion of absolute to relative paths in interpolations. But note that there are too many possibilities of absolute paths, and this conversion will not work for every possible case.

[mauvilsa]

@carmocca any comment related to #774?

[carmocca]

Seems to work well in my brief test. I can test more extensively when there's a wheel to deploy