Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable instantiations without resolution #2268

Open
kpoeppel opened this issue Jun 20, 2022 · 10 comments · May be fixed by #2269
Open

Enable instantiations without resolution #2268

kpoeppel opened this issue Jun 20, 2022 · 10 comments · May be fixed by #2269
Labels
enhancement Enhanvement request

Comments

@kpoeppel
Copy link

🚀 Feature Request

Add a flag _resolve_ to hydra.utils.instantiate to optionally disable the OmegaConf.resolve part.

Motivation

Sometimes one wants to add some self-references to the configuration, e.g. instantiating a logger with all the configuration.

_target_: MyLogger
cfg: ${oc.eval:'cfg'}

For this kind of instantiation it would be nice to have a _resolve_ flag, which can be set to false. Otherwise the top level of the configuration is different, which leads to resolution errors.

Pitch

Describe the solution you'd like
Adding this flag enables simple configuration self-references without infinite recursions (in conjunction with an eval resolver). As the flag should be true by default, the current usual behaviour is unchanged.

Describe alternatives you've considered
It might be possible to do this in code, but that's not the point of configurability. Already this kind of solution requires code modifications (cfg variable in the respective namespace).

Are you willing to open a pull request? (See CONTRIBUTING)
yes

@kpoeppel kpoeppel added the enhancement Enhanvement request label Jun 20, 2022
@kpoeppel kpoeppel linked a pull request Jun 20, 2022 that will close this issue
@Jasha10
Copy link
Collaborator

Jasha10 commented Jun 21, 2022

Hi @kpoeppel,
I don't understand your use-case. Could you please provide a more complete example?

@24hours
Copy link

24hours commented Jun 21, 2022

I faced similar problem as well

considering the script below

func:
    _target_: my_module.train

# Only able to discover num_class during runtime
num_classes: -1

model:
   num_class: ${num_class}
def train(cfg):
   # assuming ret = 10
   cfg.num_class = automatic_detect_num_class()

   assert cfg.model.num_class == 10
   # False, because call resolved all interpolation

@hydra.main(config_path="configs", config_name="config", version_base="1.2")
def main(cfg: DictConfig) -> None:
    # dynamically trigger function based on config
    call(cfg.func, cfg=cfg)

cfg.func could be any training function depending on user override. While in this case we should run different script instead of relying on cfg.func. Our actual use case compose multiple cfg.func to start an experiment. Now our solution is wrapping cfg in some class to prevent omegaconf.resolve being applied.

@kpoeppel
Copy link
Author

kpoeppel commented Jun 21, 2022

Image you have an experiment that you want to use multiple loggers with, in a configurable way. The call or instantiate is a perfect fit here, in case you do not want to add additional specific logic to your source code.

def construct_loggers(cfg):
    loggers = []
    for logger in cfg.loggers:
        loggers.append(hydra.utils.instantiate(logger, cfg=cfg, _recursive_=False))

You now want to hand over the complete configuration (resolved once) to the logger. The problem now is, that cfg is integrated into the DictConfig of logger, which sets a different top level, so resolution doesn't work anymore.
The same happens if you add an unresolved yaml-string of the configuration, which might be another way of doing it.

@24hours : Thanks for the idea for a workaround.
Edit: Change recursive to the correct _recursive.

@Jasha10 Jasha10 linked a pull request Jun 26, 2022 that will close this issue
@Jasha10
Copy link
Collaborator

Jasha10 commented Jun 26, 2022

The call or instantiate is a perfect fit here, in case you do not want to add additional specific logic to your source code.

def construct_loggers(cfg):
    loggers = []
    for logger in cfg.loggers:
        loggers.append(hydra.utils.instantiate(logger, cfg=cfg, recursive=False))

Let me make sure I understand correctly:
Is the above construct_loggers function an example of the "additional specific logic" that you want to avoid adding to the source code?

@kpoeppel
Copy link
Author

The call or instantiate is a perfect fit here, in case you do not want to add additional specific logic to your source code.

def construct_loggers(cfg):
    loggers = []
    for logger in cfg.loggers:
        loggers.append(hydra.utils.instantiate(logger, cfg=cfg, recursive=False))

Let me make sure I understand correctly: Is the above construct_loggers function an example of the "additional specific logic" that you want to avoid adding to the source code?

I am sorry that my example changed slightly from the original post to the first reply. The goal is to use the configuration to choose among loggers, that take the unresolved configuration as an argument.
In this sense there is a potential infinite recursion or a change of the configuration root, which blocks a proper resolution.
The shown logic is actually something one would like to keep out (in case I have loggers that take the configuration as a differently named argument or not at all).
As shown in the original pitch, this might be possible using a custom resolution and a global variable. Let me give a complete example:

# config.yaml
logger:
  _target_: main.LoggerA
  cfg: ${oc.eval:'OmegaConf.to_yaml(cfg)'}
# main.py
from omegaconf import OmegaConf, DictConfig
import hydra
from typing import Optional


cfg: Optional[DictConfig] = None

# main.py
class LoggerA:
    def __init__(self, cfg):
        self.cfg = cfg
        # .. do some logging


def log(cfg):
    logger = hydra.utils.instantiate(cfg.logger, _recursive_=False, _resolve_=False)
    return logger


def oc_eval(expr, _globals, _locals):
    try:
        # print("EvalExpr", expr, _globals, _locals)
        return eval(expr, _globals, _locals)
    except BaseException as err:
        print(err)
        return None


@hydra.main(config_path=".", config_name="config")
def main(conf: DictConfig) -> None:
    global cfg
    OmegaConf.register_new_resolver("oc.eval", lambda expr: oc_eval(expr, globals(), locals()))
    cfg = conf
    OmegaConf.resolve(conf)
    print(conf)
    log(cfg)
    
if __name__ == "__main__":
    main()

@Jasha10
Copy link
Collaborator

Jasha10 commented Jul 2, 2022

I see.
Note that resolving ${oc.eval:'OmegaConf.to_yaml(cfg)'} results in a string that contains "${oc.eval:'OmegaConf.to_yaml(cfg)'}" as a substring. This means resolving the config multiple times will result in a longer and longer config.
For example:

from omegaconf import OmegaConf


def oc_eval(expr, _globals, _locals):
    try:
        # print("EvalExpr", expr, _globals, _locals)
        return eval(expr, _globals, _locals)
    except BaseException as err:
        print(err)
        return None


OmegaConf.register_new_resolver(
    "oc.eval", lambda expr: oc_eval(expr, globals(), locals())
)

cfg = OmegaConf.create(
    """
logger:
  cfg: ${oc.eval:'OmegaConf.to_yaml(cfg)'}
"""
)

print(f"CONFIG 0: {cfg}")
OmegaConf.resolve(cfg)
print(f"CONFIG 1: {cfg}")
OmegaConf.resolve(cfg)
print(f"CONFIG 2: {cfg}")
OmegaConf.resolve(cfg)
print(f"CONFIG 3: {cfg}")

Running this script results in the following output:

python main.py
CONFIG 0: {'logger': {'cfg': "${oc.eval:'OmegaConf.to_yaml(cfg)'}"}}
CONFIG 1: {'logger': {'cfg': "logger:\n  cfg: ${oc.eval:'OmegaConf.to_yaml(cfg)'}\n"}}
CONFIG 2: {'logger': {'cfg': 'logger:\n  cfg: logger:\n  cfg: "logger:\\n  cfg: ${oc.eval:\'OmegaConf.to_yaml(cfg)\'}\\n"\n\n'}}
CONFIG 3: {'logger': {'cfg': 'logger:\n  cfg: logger:\n  cfg: "logger:\\n  cfg: logger:\n  cfg: "logger:\\n  cfg: logger:\\n  cfg: \\"logger:\\\\n  cfg: ${oc.eval:\'OmegaConf.to_yaml(cfg)\'}\\\\\\\n    n\\"\\n\\n"\n\\n"\n\n'}}

This feels fairly pathological to me.

@Jasha10
Copy link
Collaborator

Jasha10 commented Jul 2, 2022

How about the following as a workaround?

def log(cfg):
    logger = hydra.utils.instantiate(cfg.logger, cfg=cfg)  # pass cfg explicitly
    return logger

@Jasha10
Copy link
Collaborator

Jasha10 commented Jul 2, 2022

I faced similar problem as well

Interesting. Thanks for the additional datapoint @24hours!

@kpoeppel
Copy link
Author

How about the following as a workaround?

def log(cfg):
    logger = hydra.utils.instantiate(cfg.logger, cfg=cfg)  # pass cfg explicitly
    return logger

This can only be used with _recursive_=False. But otherwise it works.

@Jasha10
Copy link
Collaborator

Jasha10 commented Nov 6, 2022

Cross link to issue #1758, which is also related to problems with OmegaConf.resolve during instantiation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhanvement request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants