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

Command line interpolations are not being evaluated lazily, leading to changed multirun behaviour #725

Closed
rohitgirdhar opened this issue Jun 26, 2020 · 8 comments · Fixed by #734
Labels
bug Something isn't working
Milestone

Comments

@rohitgirdhar
Copy link

In v0.11, when I wanted to do multirun with certain parameters dependent on each other, I would do something like

./train.py param1=1,2,3 param2=\${param1} -m

and this would launch 3 jobs, with same values for param1 and param2.

With v1.0, this seems to be launching 3*3 = 9 jobs, with all possible combinations of param1 and param2. Wondering if that's expected behavior? My understanding was that the interpolations are evaluated lazily, so it should have the same behavior as in v0.11.

System information

  • Hydra Version : 1.0rc1
  • Python version : python 3.7.7
  • Virtual environment type and version : conda 4.7.10
  • Operating system : Linux
@rohitgirdhar rohitgirdhar added the bug Something isn't working label Jun 26, 2020
@rohitgirdhar
Copy link
Author

rohitgirdhar commented Jun 27, 2020

Here is my actual usecase: basically I have been using this functionality to couple variables, for eg, if I am running a model on 5 folds, and I want to load a pre-trained model corresponding to the specific fold, I would do something like this:

./train.py fold_id=0,1,2,3,4 pretrained=/checkpoint/\${fold_id}

@omry omry added this to the 1.0.0 milestone Jun 27, 2020
@omry
Copy link
Collaborator

omry commented Jun 27, 2020

will take a look, for now you can use this as a possible workaround:

Use a config group with this:

things/model_per_fold.yaml

fold_id: ???
pretrained: /checkpoint/${fold_id}

and then from the command line do:

$ python foo.py +things=model_per_fold fold_id=0,1,2,3,4

@immanuelweber
Copy link

immanuelweber commented Jul 2, 2020

I'm experiencing an issue probably similar to this. Please let me know if it's better to open a separate issue for the following or if relates to the one above:
I'm playing around with overrides on the command line and a very simple example is this:
config file:

a: 10
b: 20

script:

import hydra
from omegaconf import DictConfig

@hydra.main(config_name="config")
def run(config: DictConfig):
    print(hydra.__version__)
    print(config.pretty(), config.a, config.b)


if __name__ == "__main__":
    run()

if I call it like:
python test.py a=42 b=${a}

the output is:

1.0.0rc2
a: 42
b: ${a}
 42 42

if I call it with multirun:
python test.py -m a=42 b=${a}

the output (note the difference in the pretty print, now b is not handled lazily):

1.0.0rc2
a: 42
b: 42
 42 42

but if I have a real multirun:
python test.py -m a=42,23 b=${a}

now b is not interpolated and not overriden:

[2020-07-02 19:00:02,938][HYDRA] Launching 2 jobs locally
[2020-07-02 19:00:02,939][HYDRA]        #0 : a=42 b=10
1.0.0rc2
a: 42
b: 10
 42 10
[2020-07-02 19:00:03,427][HYDRA]        #1 : a=23 b=10
1.0.0rc2
a: 23
b: 10
 23 10

@immanuelweber
Copy link

immanuelweber commented Jul 2, 2020

I played around a bit more:
if "a" is not part of the config file and I do this:
python test.py -m +a=42,23

i get the following error:

omegaconf.errors.ConfigKeyError: str interpolation key 'a' not found
        full_key: hydra.overrides.task[1]
        reference_type=List[str]
        object_type=list

with just +a=42 it works with and without -m.

@immanuelweber
Copy link

ok, I just realized that you have a pull request out for this already. Sorry for the noise :)

@omry
Copy link
Collaborator

omry commented Jul 3, 2020

I am working on it, the pull request is pending some deeper changes.
See the document linked from #738 to context.

@immanuelweber
Copy link

Small update:
I pip installed this pull request and the overrides are now lazy. This made some things so beautiful now :)
There pops up another issue: In my actual code I have an interpolation to ${hydra:job.override_dirname}
Now, this can not be resolved in the sweep runs anymore:
my modified minimal example from above:

import hydra
from omegaconf import DictConfig

@hydra.main(config_path="conf", config_name="config")
def run(config: DictConfig):
    print(hydra.__version__)
    print(config.pretty(), config.a, config.b, config.c)


if __name__ == "__main__":
    run()

modified config.yaml:

a: 10
b: 20

c: ${hydra:job.override_dirname}

results when called with python test.py -m a=42,23 b=${a} or python test.py -m b=${a} in:

Exception has occurred: ConfigKeyError
str interpolation key 'a' not found
	full_key: hydra.job.override_dirname
	reference_type=Optional[HydraConf]
	object_type=HydraConf

I circumvented this temporarily by using this in my actual code:

hdr = HydraConfig.get()
override_dirname= hdr.job.override_dirname

@omry
Copy link
Collaborator

omry commented Jul 3, 2020

@egonuel thanks.
the PR does not pass tests and is not ready. you are using it at your own risk :)

I am not ready to push additional bugs onto this stack just yet.
Please wait until I land what I am working on now and if there are still issues you can file new bugs.

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
Development

Successfully merging a pull request may close this issue.

3 participants