add --experimental-rerun CLI option #2098
Conversation
facebook-github-bot
added
the
CLA Signed
hydra/main.py
Outdated
| cfg = _get_rerun_conf(args.experimental_rerun, args.overrides) | ||
| ret = task_function(cfg) | ||
| _flush_loggers() | ||
| return ret |
There was a problem hiding this comment.
Why return the value here? The else branch below does not return anything.
There was a problem hiding this comment.
return here to be consistent with the cfg_passthrough case above, since this feels more like a special case of cfg_passthrough...
There was a problem hiding this comment.
What if we support --experimental-rerun in combination with the --multirun flag in the future (so that users could pass a sweep override)? It's not clear what the return value would be. This is the reason why the else branch below does not return a value (see the comment on that branch).
There was a problem hiding this comment.
Good point. multirun is going to be way more complex than what is purposed here. And as I mentioned here I think it makes sense to only support SINGLE run for now and see how it goes. I should have make it more clear on what this issue is trying to address, I've updated the PR description.
I will remove the return statement since I agree it sees redundant.
|
One additional note: If this feature becomes non-experimental in the future, we might want to polish this interaction with tab-completion. |
I cannot seem to repro this. Do you mean this does not play well with the shell completion plugin Hydra offers? |
Yes, exactly. |
hydra/main.py
Outdated
| cfg = _get_rerun_conf(args.experimental_rerun, args.overrides) | ||
| ret = task_function(cfg) | ||
| _flush_loggers() | ||
| return ret |
There was a problem hiding this comment.
What if we support --experimental-rerun in combination with the --multirun flag in the future (so that users could pass a sweep override)? It's not clear what the return value would be. This is the reason why the else branch below does not return a value (see the comment on that branch).
Gotcha, yea, I can work on that as a follow up item. |
news/1805.feature
Outdated
| @@ -0,0 +1 @@ | |||
| Add `--experimental-rerun` command-line option | |||
There was a problem hiding this comment.
... to reproduce pickled single runs
| ### Important Notes | ||
| This is an experimental feature. Please reach out if you have any question. | ||
| - Only single run is supported. | ||
| - `--experimental-rerun` cannot be used with other command-line options or overrides. They will simply be ignored. |
There was a problem hiding this comment.
We should at least warn about ignored args.
There was a problem hiding this comment.
edited the warning message in b6ae6ed
|
The implementation looks good, exept perhaps warning about ignored args with --experimental-rerun. |
related to #1805
The goal is to add experimental support for single RUN only for now. We will iterate depending on the feedback.