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

[Feature Request] Retain the final defaults list in the Hydra config #956

Closed
nmichlo opened this issue Sep 9, 2020 · 6 comments · Fixed by #1170
Closed

[Feature Request] Retain the final defaults list in the Hydra config #956

nmichlo opened this issue Sep 9, 2020 · 6 comments · Fixed by #1170
Labels
enhancement Enhanvement request
Milestone

Comments

@nmichlo
Copy link

nmichlo commented Sep 9, 2020

🚀 Feature Request

Add an OmegaConf resolver for defaults, or allow value interpolation for defaults globally in a config by disabling deletion of the defaults key when Hydra loads a config.

tl;dr: add a defaults resolver, similar to the hydra resolver.

Motivation

Is your feature request related to a problem? Please describe.

Currently defaults can only be accessed from within the defaults list, as this portion of the config is deleted after being loaded by hydra.

For example, this currently works:

# config.yaml
defaults:
  - dataset: imagenet
  - model: alexnet
  - dataset_model: ${defaults.0.dataset}_${defaults.1.model}  # this works!
    optional: true

However, the following does not work:

# config.yaml
defaults:
  - dataset: imagenet
  - model: alexnet
dataset_model_name: ${defaults.0.dataset}_${defaults.1.model}  # this breaks!

This is not ideal when you want to obtain the name of a specific default to be used for example when you are logging.

  • Currently (as far as I know) to get around this, you can specify a variable within each config in the group that corresponds to the name of the file which can then be used for value interpolation globally. This is not convenient due to duplication of the name which the file is already called and means you have to nest a _target_ that could otherwise be at the root of that config, but can't due to parameter conflicts.

The current inconvenient way around this is:

# dataset/imagenet.yaml
name: imagenet  # this is the same as the filename!
cls:            # this is inconvenient!
  _target_: ImagenetDataset
...

# model/alexnet.yaml
name: alexnet  # this is the same as the filename!
cls:           # this is inconvenient!
  _target_: AlexnetModel
...

# config.yaml
defaults:
  - dataset: imagenet
  - model: alexnet
dataset_model_name: ${dataset.name}_${model.name}  # this works!

The proposed way to do this would be:
However, the following does not work:

# dataset/imagenet.yaml
_target_: ImagenetDataset
...

# model/alexnet.yaml
_target_: AlexnetModel
...

# config.yaml
defaults:
  - dataset: imagenet
  - model: alexnet

# proposed solution, see below.
dataset_model_name: ${defaults:0.dataset}_${defaults:1.model}  # this resolver does not yet exist!

# alternate solution, see below.
dataset_model_name_alt: ${defaults.0.dataset}_${defaults.1.model}  # this breaks!

Pitch

Describe the solution you'd like

  • Add a new custom resolver similar to hydra (eg. ${hydra:job.name}), this new resolver would be called defaults and so to access the deleted ${defaults.0.dataset} you could use ${defaults:0.dataset}

Describe alternatives you've considered

  • Add a parameter to hydra.main that can be set to: preserve_defaults=True to stop the defaults key being deleted from the config when loaded by Hydra.

Are you willing to open a pull request? (See CONTRIBUTING)
Unfortunately not, I do not have experience with the codebase.

Additional context

No additional context.

@nmichlo nmichlo added the enhancement Enhanvement request label Sep 9, 2020
@omry
Copy link
Collaborator

omry commented Sep 10, 2020

I can add the final defaults list to the Hydra config object, which will allow you to interact with it.
Currently the override are retained but not the final defaults list.

see hydra.overrides in your Hydra config.

@omry omry changed the title [Feature Request] OmegaConf resolver for defaults [Feature Request] Retain the final defaults list in the Hydra config Sep 10, 2020
@omry omry added this to the 1.1.0 milestone Sep 10, 2020
@nmichlo
Copy link
Author

nmichlo commented Sep 10, 2020

Thank you. I missed hydra.overrides and now I realise I've been talking about defaults the whole time without considering them, the final merged defaults + overrides list as you say makes much more sense.

@omry
Copy link
Collaborator

omry commented Sep 10, 2020

Great. I will address this with 1.1, as small a change as it is I am done adding new features to 1.0.

@omry
Copy link
Collaborator

omry commented Dec 16, 2020

#1170 is adding a new fields to the hydra config:

hydra:
  choices:
     group: choice
     ...

This can be used in interpolation like:

defaults:
  - group: bar

key: ${hydra:choices.group}  # bar, or any other choice if overridden.

It also supports nested defaults (group1/group2) and config groups with overridden packages (group1@foo).

@turian
Copy link

turian commented Dec 19, 2020

@omry 291 files changed in #1170 ? Wow you're a beast :)

I am curious if there is more detailed documentation for upcoming recursive defaults. Thank you.

@omry
Copy link
Collaborator

omry commented Dec 19, 2020

#1170 is the biggest single feature in a single PR in Hydra. It's easily double in size of the second place.

If you look at the long todo list in that PR you will see that documentation changes is still not done.
For now read the design doc linked to from #1170, it's very well written and it very close to being a user guide.

@omry omry closed this as completed in #1170 Jan 1, 2021
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