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

Recursive defaults support #171

Closed
omry opened this issue Sep 28, 2019 · 10 comments · Fixed by #1044 or #1170
Closed

Recursive defaults support #171

omry opened this issue Sep 28, 2019 · 10 comments · Fixed by #1044 or #1170
Labels
enhancement Enhanvement request
Milestone

Comments

@omry
Copy link
Collaborator

omry commented Sep 28, 2019

Hydra does not currently support a recursive defaults list. the defaults are only specified at the primary config file.
There is some special support for Hydra itself, but this can be ignored for the purpose of this issue.

Recursive defaults list are tricky, it's important to ensure intuitive and consistent order of merging.

Proposed logic

if the defaults in the primary file are in that order:
main.yaml

defaults:
  - aa: x1
  - bb: x2

The relative order should be preserved. it is however possible for other files to be injected in between aa and bb, for example if aa/x1 had it's own defaults:

aa/x1.yaml

defaults:
  - cc: x3
  - dd: x4

the resulting defaults list might looks like:

  - aa: x1
  - cc: x3  # from aa/x1 
  - dd: x4  # from aa/x1 
  - bb: x2

Let's look at a more complicated case:

main.yaml

defaults:
  - aa: x1
  - bb: x2
  - cc: must_win

aa/x1.yaml

defaults:
  - cc: x3
  - dd: x4

In this case, the result should be:
the resulting intermediate list will looks like:

  - aa: x1
  - cc: x3
  - dd: x4
  - bb: x2
  - cc: must_win

but the final result will be:

  - aa: x1
  - cc: must_win
  - dd: x4
  - bb: x2

Note how cc retains it's order in the list, but got the last value.

The rule here is:

  1. Composition order determined by the first time a configuration group is introduced (the db in db=mysql).
  2. Composition choice (the mysql in db=mysql) is determined by the last mention of the group).
@omry omry added the enhancement Enhanvement request label Sep 28, 2019
@omry omry changed the title Support merging multiple default lists Recursive defaults support Mar 18, 2020
@omry omry added this to the 1.1.0 milestone May 10, 2020
@Queuecumber
Copy link
Contributor

Would like to see this, I have a bunch of options for config groups, when I find the ones that work well using command line overrides I'd like to dump them into a config file so I don't have to worry about the long command line again.

@omry
Copy link
Collaborator Author

omry commented Sep 21, 2020

The plan is to implement this for 1.1.

@ppwwyyxx
Copy link

ppwwyyxx commented Oct 20, 2020

How does recursive defaults work with package overrides? e.g. something like this:

# model/maskrcnn.yaml:
# @package _group_
a: 1
b: 2

# model/maskrcnn2.yaml:
# @package _group_
b: 3
c: 4

defaults:
  - model/maskrcnn   # ??
  - _self_

# main.yaml:
defaults:
  - [email protected]: maskrcnn2

@omry
Copy link
Collaborator Author

omry commented Oct 21, 2020

The key for overriding becomes group@package is you have a package override.
This should work in a similar way to how package overrides are handled from the command line.

@ppwwyyxx
Copy link

I can't seem to make the above example work with the commit that's just added. I ended up with

{'model': {'a': 1, 'b': 2, 'submodule': {'b': 3, 'c': 4}}}

@omry
Copy link
Collaborator Author

omry commented Oct 30, 2020

Looks like you are adding the model in two places.
If you want to rename the existing model, you need to use the rename syntax.

This is documented here:
https://github.com/facebookresearch/hydra/blob/master/website/docs/advanced/defaults_list.md

(Unfortunately this is not in the website yet due to some issue we are looking into).

If you think you have found a bug or have a question about the recursive defaults open a new issue.

@omry
Copy link
Collaborator Author

omry commented Oct 30, 2020

You can look at the resulting defaults list (and some other info with --info).

@omry
Copy link
Collaborator Author

omry commented Oct 30, 2020

I am not ruling out a bug though, play with it and if you think there is a bug open a new issue with a minimal example, along with what config you are trying to compose (which is missing here).

@mwillwork
Copy link

@omry is there an ETA for this feature and for Hydra 1.1?

@omry
Copy link
Collaborator Author

omry commented Dec 21, 2020

This feature (or more accurately the one from #1170 a complete rewrite that is not merged yet) is more or less ready (mostly only documentation and some followups are missing).

There are a number of other things which are also planned for Hydra 1.1.
You can follow the milestone for 1.1 here.

I am guestimating that initial release candidate will be ready sometimes during H1 2021.

Keep in mind that Hydra 1.1 will take even longer to hit fbcode due to the monorepo nature of it.

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
4 participants