facebookresearch / hydra

Hydra is a framework for elegantly configuring complex applications
https://hydra.cc
MIT License
8.82k stars 635 forks source link

[Feature Request] Add resolve config under `.hydra/config_resolve.yaml` by default. #2254

Open talregev opened 2 years ago

talregev commented 2 years ago

🚀 Feature Request

As you added this feature: https://github.com/facebookresearch/hydra/issues/1585

Hydra by default are creating under .hydra/config.yaml a config that it not resolve. That great! But we want to add (additional and not instead). a config: .hydra/config_resolve.yaml. that will be resolve of the config.yaml similar if I run --cfg job --resolve in the cli.

Thank you in advanced.

Jasha10 commented 2 years ago

Resolving the config could be an expensive operation (depending on the use-case). For example, if you use custom resolvers, there could be arbitrary computation involved in resolving the config. For this reason, I don't think we should resolve the config by default.

You can resolve and save the config manually:

...

@hydra.main(...)
def my_app(cfg):
    OmegaConf.resolve(cfg)
    OmegaConf.save(cfg, "filepath.yaml")
    ...

You could also use a Hydra Callaback (e.g. on_job_start) to resolve+save the config before your job runs.

talregev commented 2 years ago

Resolving the config could be an expensive operation (depending on the use-case). For example, if you use custom resolvers, there could be arbitrary computation involved in resolving the config. For this reason, I don't think we should resolve the config by default.

You can resolve and save the config manually:

...

@hydra.main(...)
def my_app(cfg):
    OmegaConf.resolve(cfg)
    OmegaConf.save(cfg, "filepath.yaml")
    ...

You could also use a Hydra Callaback (e.g. on_job_start) to resolve+save the config before your job runs.

Can we add a flag '--resolve' that will add and not replace this config?

jieru-hu commented 2 years ago

Can we add a flag '--resolve' that will add and not replace this config?

@talregev - sorry I'm not sure what you mean here

But I agree with Jasha, this is a good use case for Hydra callback and should be straightforward to implement. Have you looked into that?

obust commented 2 years ago

Alternatively, is there a way to resolve once we loaded the config

outdir = Path.cwd()

cfg = OmegaConf.merge(
    OmegaConf.load(outdir / '.hydra' / 'hydra.yaml'),
    OmegaConf.load(outdir / '.hydra' / 'config.yaml'))

OmegaConf.resolve(cfg)

but this fails with the following error

...
File /usr/local/conda/lib/python3.8/site-packages/omegaconf/omegaconf.py:435, in OmegaConf.register_new_resolver.<locals>.resolver_wrapper(config, parent, node, args, args_str)
    432 if pass_root:
    433     kwargs["_root_"] = config
--> 435 ret = resolver(*args, **kwargs)
    437 if use_cache:
    438     cache[args_str] = ret

File /usr/local/conda/lib/python3.8/site-packages/hydra/core/utils.py:193, in setup_globals.<locals>.<lambda>(path)
    183 def setup_globals() -> None:
    184     # please add documentation when you add a new resolver
    185     OmegaConf.register_new_resolver(
    186         "now",
    187         lambda pattern: datetime.now().strftime(pattern),
    188         use_cache=True,
    189         replace=True,
    190     )
    191     OmegaConf.register_new_resolver(
    192         "hydra",
--> 193         lambda path: OmegaConf.select(cast(DictConfig, HydraConfig.get()), path),
    194         replace=True,
    195     )
    197     vi = sys.version_info
    198     version_dict = {
    199         "major": f"{vi[0]}",
    200         "minor": f"{vi[0]}.{vi[1]}",
    201         "micro": f"{vi[0]}.{vi[1]}.{vi[2]}",
    202     }

File /usr/local/conda/lib/python3.8/site-packages/hydra/core/hydra_config.py:31, in HydraConfig.get()
     29 instance = HydraConfig.instance()
     30 if instance.cfg is None:
---> 31     raise ValueError("HydraConfig was not set")
     32 return instance.cfg.hydra

InterpolationResolutionError: ValueError raised while resolving interpolation: HydraConfig was not set
Jasha10 commented 2 years ago

but this fails with the following error

@obust see issue #1786 and my comment on issue #2017.