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

[Feature Request] Permit user-defined arguments if needed #215

Closed
withsmilo opened this issue Oct 9, 2019 · 27 comments
Closed

[Feature Request] Permit user-defined arguments if needed #215

withsmilo opened this issue Oct 9, 2019 · 27 comments
Labels
enhancement Enhanvement request wontfix This will not be worked on

Comments

@withsmilo
Copy link

withsmilo commented Oct 9, 2019

🚀 Feature Request

Motivation

Is your feature request related to a problem? Please describe.

Because hydra defines argparse.ArgumentParser internally, user cannot add own new parser.

Pitch

Describe the solution you'd like

I added a new param(user_arg_parser) to the hydra.main decorator. Then my_app function can receive the additional args param passed through Hydra.

parser = argparse.ArgumentParser(add_help=False)
parser.add_argument("--user_arg", type=str, help="This is user's argument")

@hydra.main(config_path="config/config.yaml", user_arg_parser=parser)
def my_app(cfg, args):
    print(cfg.pretty())

Describe alternatives you've considered

Are you willing to open a pull request? (See CONTRIBUTING)
Yes.

Additional context

Add any other context or screenshots about the feature request here.
You can also record a video

@withsmilo withsmilo added the enhancement Enhanvement request label Oct 9, 2019
@omry
Copy link
Collaborator

omry commented Oct 9, 2019

Hi,
One of the big points of Hydra is that you do not need to declare args for argparse.
You can put your args in a config file and override freely.
What is the actual use case here?

@omry
Copy link
Collaborator

omry commented Oct 9, 2019

Basically, if you are trying to port an existing application that is using ArgParse to use Hydra, you should remove argparse.

If you can describe your use case in more details I can give some advice on how to do it.
feel free to join the Google group and ask for help there.

Close this issue as this is a fundamental design choice that is not going to change.

@omry omry added the wontfix This will not be worked on label Oct 9, 2019
@omry omry closed this as completed Oct 9, 2019
@withsmilo
Copy link
Author

withsmilo commented Oct 9, 2019

@omry : Thanks for your kindly reply. But what if I want to receive a certain command as well as configurations through argument? For example,

python my_app.py db.user_id=test —command push ...

@omry
Copy link
Collaborator

omry commented Oct 9, 2019

you can add a command to your config, for example:

config.yaml:

command: default_command
# or, if you want to require the user to specify the command in the command line:
# command: ???

Once you do that, you can call your app like:
python my_app.py db.user_id=test command=push

@withsmilo
Copy link
Author

Great suggestion! but what should I do if I want to add a description of the command to help menu? Users will habitually call ‘—help’ to get a guide for the command.

@omry
Copy link
Collaborator

omry commented Oct 9, 2019

At the moment the closest thing to this is to use -c to print the configuration.

See this for info:
https://cli.dev/docs/tutorial/debugging#printing-the-configuration

I am planning to improve this though.

@omry
Copy link
Collaborator

omry commented Oct 9, 2019

See issue #1.
There are also some ongoing work that will take longer to allow annotating config nodes. one of the annotation I plan is to allow describing a node, setting the type, maybe a list of possible values etc.

For now you can do most of it manually, for example:

assert cfg.command in ('push', 'pull'')

@withsmilo
Copy link
Author

@omry : Thanks a lot for your detail guide! I'm trying to apply hydra to my Python application. 😁

@omry
Copy link
Collaborator

omry commented Oct 9, 2019

Awesome.
Please join the Google group if you want to ask more questions or want to share your success/failure applying Hydra to your application with others.

@omry
Copy link
Collaborator

omry commented Oct 11, 2019

Btw, I started to work on #1, It will be possible to create fully custom application help in the next Hydra release.

@withsmilo
Copy link
Author

Great news! I finished to migrate my Python app using hydra. I’m following you.

@omry
Copy link
Collaborator

omry commented Oct 11, 2019

awesome.
Can you share some info about your app?

@withsmilo
Copy link
Author

I'm developing the in-house ML platform, which is in charge of pipelining, training, deploying and real-time serving. And I'm refactoring the legacy version of the commander /w conf. management ML tool using hydra for now. I think hydra is really awesome package fit for our purpose. Thanks a lot!

@omry
Copy link
Collaborator

omry commented Oct 11, 2019

This sounds like a great use case!
I hope your users will be happy and will start using Hydra for their own projects as well :)

@omry
Copy link
Collaborator

omry commented Oct 11, 2019

By the way:
Hydra has a launcher abstraction that can be used to launch jobs to a remote cluster.
It might make sense for you to implement such a plugin if you have a local cluster.

If you want to try, I can give you some pointers (but let's move the discussion to the Google group, I want others to know this is an option).

@withsmilo
Copy link
Author

It seems to be very useful for us because we have separated training / serving clusters. However, in official document, I can't find how to launch a job to run remotely. Could you share some sample codes to do it?

@omry
Copy link
Collaborator

omry commented Oct 11, 2019

This is not well documented right now because there isn't any public plugin that can do it at the moment.
There will be one based on Ray when #152 is done. #152 also got some reference examples.
Ray should be able to launch for some public cloud platforms. but writing a Launcher plugin shouldn't be very hard (depending on the API you have to launch to your internal cluster).

@omry
Copy link
Collaborator

omry commented Oct 11, 2019

What system are you using to launch jobs to your cluster(s)?
Is it based on anything public like Slurm?

@withsmilo
Copy link
Author

Thanks for sharing your history. If hydra uses Ray to launch a job to run remotely, do we have to install the Ray cluster over our cluster? I think maybe yes.

What system are you using to launch jobs to your cluster(s)?

In our case, we use basically Airflow + own commander/conf tool.

@omry
Copy link
Collaborator

omry commented Oct 11, 2019

The idea is that there can be a plugin to launch with Airflow.
I am relying on the community (you!) to help build those plugins, but I can help with advice and examples.

@withsmilo
Copy link
Author

@omry
I need to go digging to hydra now before implementing a plugin to launch something. So I tried an interesting work this weekend. https://github.com/withsmilo/When-ML-pipeline-meets-Hydra

@omry
Copy link
Collaborator

omry commented Oct 12, 2019

very nice.
One thing you are not talking about is that you can also override the composed configuration's values from the command line.

@withsmilo
Copy link
Author

I will add the mentioned description to my app as soon as possible. Thanks!

@jungerm2
Copy link

I'm not sure if this is the place to add to this, but it seems to me that there's some value in allowing user-defined parsers. Specifically, consider this use-case:

An ML application that has a single endpoint, say run.py to do both inference and training. These two options should be in a mutually exclusive group (as defined by argparse). By instead adding them to the config file, there's no way to enforce this.
Another simple use case would be to allow users to specify a default conf path instead of having @hydra.main(config_path="conf/config.yaml") hardcoded.

What work-around would you suggest?

@omry
Copy link
Collaborator

omry commented Feb 23, 2020

@jungerm2:
There is a different issue for allowing a different config file: #386 .
As for tow different run modes:
If those are really mutually exclusive, the work around for now is to have two different entry point.
train.py:

@hydra.main(config_path="train.yaml")
def train(cfg):
  ..

test.py:

@hydra.main(config_path="test.yaml")
def test(cfg):
  ..

If you actually do want to share most of the config, you create a mode parameter in your config:

config.yaml:

defaults:
  - model: foo
  - dataset: bar

mode: train 

then the code can behave differently based on the mode.

@omry omry reopened this Feb 23, 2020
@omry omry closed this as completed Feb 23, 2020
@jungerm2
Copy link

Thank you that makes sense. In the case where we have multiple (2+) mutually exclusive args, multiple entry points is probably better design anyways...

@tarohi24
Copy link

tarohi24 commented Oct 8, 2024

I'm not sure if adding a custom CLI option is supported at this point. Anyway here is my code for this by lazily calling the hydra.main decorator.

import argparse
import sys

import hydra


def main(cfg):
    ...


def _main():
    parser = argparse.ArgumentParser()
    parser.add_argument("--custom_option", type=int)
    args, unknown = parser.parse_known_args()
    # do some pre-processing
    sys.argv = [sys.argv[0]] + unknown
    hydra.main(version_base=None, config_path=".", config_name="default")(main)()


if __name__ == "__main__":
    _main()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhanvement request wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants