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

[Bug] issues with named arguments in hydra.utils.instantiate #400

Closed
stepelu opened this issue Feb 7, 2020 · 1 comment · Fixed by #404
Closed

[Bug] issues with named arguments in hydra.utils.instantiate #400

stepelu opened this issue Feb 7, 2020 · 1 comment · Fixed by #404
Labels
bug Something isn't working
Milestone

Comments

@stepelu
Copy link

stepelu commented Feb 7, 2020

hydra.utils.instantiate converts to OmegaConf named arguments, making it impossible to use the named arguments syntax for arguments which cannot be represented in OmegaConf.

To reproduce:

conf.yaml:

class: torch.optim.Adam
params:
  lr: 1E-4

test.py:

import hydra
import torch
import torchvision

@hydra.main(config_path="conf.yaml")
def my_app(cfg):
    print(cfg.pretty())
    model = torchvision.models.alexnet()
    hydra.utils.instantiate(cfg, model.parameters())  # OK
    hydra.utils.instantiate(cfg, params=model.parameters())  # FAILS

if __name__ == "__main__":
    my_app()

** Stack trace/error message **

ValueError: key params: generator is not a primitive type

As a side note, the fact that hydra.utils.instantiate takes as input an argument named config makes it impossible to instantiate an object which expects the same argument using named arguments, which seems an unnecessary restriction.

The following solves both issues:

def instantiate_f(config):
    assert config is not None, "Input config is None"
    class_name = config["class"]
    clazz = get_class(class_name)
    params = config.params if "params" in config else OmegaConf.create()
    assert isinstance(
        params, DictConfig
    ), "Input config params are expected to be a mapping, found {}".format(
        type(config.params)
    )
    params = OmegaConf.to_container(params, resolve=True)

    def f(*args, **kwargs):
        try:
            kwargs = {**params, **kwargs}
            return clazz(*args, **kwargs)
        except Exception as e:
            log.error("Error instantiating {} : {}".format(class_name, e))
            raise e

    return f

@hydra.main(config_path="conf.yaml")
def my_app(cfg):
    print(cfg.pretty())
    model = torchvision.models.alexnet()
    instantiate_f(cfg)(params=model.parameters())  # OK
    instantiate_f(cfg)(model.parameters())  # OK

if __name__ == "__main__":
    my_app()

A note in the documentation should be added explaining the behavior wrt config, as it gets resolved immediately.

@stepelu stepelu added the bug Something isn't working label Feb 7, 2020
@omry
Copy link
Collaborator

omry commented Feb 7, 2020

Thanks for the issue report. will look into addressing it in a bit cleaner when when I have a chance.
support for passthrough was first added exactly for this use case (optimizer taking parameters()). named parameter support was added later which explains why this issue went unnoticed by me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants