-
-
Notifications
You must be signed in to change notification settings - Fork 622
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
multirun is inconsistently misinterpreting cmdline values for float as str, but only for specific (and arbitrary) range of floats [Bug] #999
Comments
Thanks for reporting, and sorry for the frustrating bug hunt.
|
I strongly recommend that you utilize Structured Configs. They would either tell you of a type error, or in this case actually convert the input to the correct type: from dataclasses import dataclass
import hydra
from hydra.core.config_store import ConfigStore
from omegaconf import MISSING
@dataclass
class Config:
lr: float = MISSING
cs = ConfigStore.instance()
# Registering the Config class with the name 'config'.
cs.store(name="config", node=Config)
@hydra.main(config_name="config")
def my_app(cfg: Config) -> None:
print(cfg)
if __name__ == "__main__":
my_app()
|
During multirun, the input you are providing is parsed, and the converted to strings that are being passed to the launcher, which in turn parses each value again. e.g:
Broken down into two floats:
Ooops, can you spot the difference? There is a bug in the parser that causes it to not recognize it as a proper float which means it's interpreted as a string. CC @odelalleau, OMG: You killed Kenny. |
My condolences to Kenny's friends and family. |
@odelalleau can you make a note to update the upcoming OmegaConf grammar with it? it probably has the same issue. |
Yup -- already done, thanks for checking |
🐛 Bug
Description
This bug just cost me about a day of frustration looking deep into tensorflow's source code before I realized it disappeared immediately when I stopped using the '--multirun' cmdline flag.
Basically, it seems like Hydra is interpreting numerical values passed via cmdline as a different type depending on the numerical value, in a way that does not raise any explicit warnings and seems arbitrary enough that I can't imagine an actual reason for its inclusion. Thus, I believe it's a bug.
Tldr;
Checklist
To reproduce
** Minimal Code/Config snippet to reproduce **
demo.py
The directory containing demo.py also contains
configs/config.yaml
When I call the script using the command:
python demo.py --multirun model.regularization.l2=1e-3 model.lr=1.0e-5 model.lr_momentum=1e-5,1e-6,1e-7,1e-8,1e-9,1e-10
I get the output:
** Stack trace/error message **
Highlighting the important info from above:
A second, contrasting example below:
e.g.
'model.lr=1.0e-5'
is the same as
model.lr=1.0e-5
Expected Behavior
I'd expect the cmdline overrides to be interpreted in a consistent way independent of the numerical value. At the moment it appears as if Hydra simply expects the user to manually convert certain numerical values after they've been parsed for the config, and the specification of which values those are is nonexistent and highly arbitrary.
My expectation, based on having read through the docs, is that the user should expect all of the following to be interpreted as float:
and expect all of the following to be interpreted as str:
System information
Please let me know if you'd like any other info/find the cause and a way to fix that doesnt require me to write a whole config verification script (especially after I just typed all this out!)
Cheers,
Jacob
The text was updated successfully, but these errors were encountered: