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

Allow "default" in interpolation when target config node does not exist #541

Closed
ppwwyyxx opened this issue Feb 12, 2021 · 3 comments · Fixed by #687
Closed

Allow "default" in interpolation when target config node does not exist #541

ppwwyyxx opened this issue Feb 12, 2021 · 3 comments · Fixed by #687
Labels
enhancement New feature or request wishlist
Milestone

Comments

@ppwwyyxx
Copy link

Currently, when the key in interpolation doesn't exist, e.g. in:

dic = DictConfig({
        "c": "${a}"
    })
print(dic.c)

it throws exception about "str interpolation key 'a' not found".
I hope there is a way to specify default values.

Describe the solution you'd like
the env: resolver already supports defaults: https://omegaconf.readthedocs.io/en/latest/usage.html#environment-variable-interpolation
and the syntax seems nice.
The above can be written as:

dic = DictConfig({
        "c": "${a,123}"
})

Describe alternatives you've considered

@omry omry added the wishlist label Feb 12, 2021
@omry
Copy link
Owner

omry commented Feb 12, 2021

Hi @ppwwyyxx.
While it may seem like a simple addition, changing something so fundamental has a very large surface area and will require careful evaluation of how it's interacting with many of the existing features (Structured Configs, config merging and so on).

This is something I can consider if there is more interest in this feature by additional users.

In the mean time, Can you motivate this more? I have never felt the need for something like this so far.

@omry omry added the enhancement New feature or request label Feb 16, 2021
@ppwwyyxx
Copy link
Author

ppwwyyxx commented Mar 5, 2021

To give an example, consider a config organization that looks like this:

output_dir: "./output"

train_job:
  # other settings
  tensorboard_output_dir: "${output_dir}"
eval_jobs:
  - evaluator: Evaluator1
    #other settings
    predictions_output_dir: "${output_dir}"
  - evaluator: Evaluator2
    predictions_output_dir: "${output_dir}"

where a task is composed of multiple jobs that should share output_dir.
This organization works, however when such a config is created by composition of separate smaller files (e.g. each describes one job), we hope that ideally each of these smaller config files can be reused on its own (e.g., if I just want to create an evaluator from a config file eval_jobs/evaluator1.yaml).

To be able to reuse this config independently, without the presence of the top-level output_dir, it's useful to give interpolation a default value of None or "./output".

@omry
Copy link
Owner

omry commented Mar 9, 2021

This is solved by Hydra using config composition.

config.yaml:

defaults:
 - base
 - trainer

base.yaml:

output_dir: "./output"

trainer.yaml:

train_job:
  # other settings
  tensorboard_output_dir: "${output_dir}"
eval_jobs:
  - evaluator: Evaluator1
    #other settings
    predictions_output_dir: "${output_dir}"
  - evaluator: Evaluator2
    predictions_output_dir: "${output_dir}"

You can also merge base.yaml into config.yaml in this case:

config.yaml:

defaults:
 - trainer

output_dir: "./output"

This way, you can override the output_dir in a single place that will effect all the different nodes that are interpolating to it.
Also, the nodes themselves can be overridden if they really need to be diverged from the default for some reason.

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

Successfully merging a pull request may close this issue.

2 participants