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: quick CLIs #390

Merged
merged 15 commits into from Dec 13, 2018

Conversation

Projects
None yet
4 participants
@benjamin-work
Copy link
Collaborator

commented Nov 20, 2018

This feature adds functionality to build quick CLIs for scripts based on skorch. The intent is to encourage users to adopt skorch by reducing the boilerplate required to make experiments reproducible.

This basically leverages the features of fire to automatically create CLIs. In combination with skorch, almost all possible model parameters are immediately available via the command line, without all the boilerplate typically required to parse the arguments. This way, things like max_epochs, lr, device, etc. are available for free. Moreover, all arguments on the module are also available as usual (say module__num_units).

Furthermore, if there are docstrings in the numpydoc format, those can be displayed via --help, again without any boilerplate code.

A restriction right now is passing complex python objects via the command line. Things like --module__nonlin 'torch.nn.RReLU(0.1, upper=0.4)' work, but if the object contains other non-primitive objects, parsing will fail.

@thomasjpfan @taketwo Please also share your views on whether you find this feature useful.

benjamin-work added some commits Nov 19, 2018

Helper functions to for CLIs with almost boilerplate.
Add a helper function parse_args that makes it very simple to build
custom CLIs. Add an example for the usage of this and extend docs.

@benjamin-work benjamin-work self-assigned this Nov 20, 2018

@benjamin-work benjamin-work requested a review from ottonemo Nov 20, 2018

benjamin-work added some commits Nov 20, 2018

Remove fire from dev requirements, install in travis.
fire is not on the conda channels, so install would fail. Also, modify
cli tests to be skipped if fire is not installed.
@thomasjpfan

This comment has been minimized.

Copy link
Contributor

commented Nov 22, 2018

This is a pretty unique feature to add to skorch. I have not seen other ML frameworks try to add a "easy to setup CLI". Since this PR basically just adds a parse_args function, this should not add much maintenance burden to the codebase.

@taketwo

This comment has been minimized.

Copy link
Contributor

commented Nov 22, 2018

I have my own utility class that helps me to minimize CLI boilerplate. It's not as advanced when it comes to automatic discovery of parameters, so I need to manually add module parameters that are of interest. On the other hand, it helps me organize my runs directory, automatically creating directories/filenames for different experiments based on the parameters.

I guess what I want to say is, sure, a CLI helper is great to have and can give a productivity boost. Unfortunately I don't have enough time to play with your proposed implementation now. Nevertheless, it looks good and relies on an established third-party library, so I only see benefits in including this.

Almost all arguments should work out of the box. Therefore, you get
command line arguments for the number of epochs, learning rate, batch
size, etc. for free. Morevoer, you can access the module paremeters

This comment has been minimized.

Copy link
@taketwo

taketwo Nov 22, 2018

Contributor
Suggested change
size, etc. for free. Morevoer, you can access the module paremeters
size, etc. for free. Moreover, you can access the module parameters
fire.Fire(main)
This even works if your neural net is part of an sklearn pipeline, in

This comment has been minimized.

Copy link
@taketwo

taketwo Nov 22, 2018

Contributor
Suggested change
This even works if your neural net is part of an sklearn pipeline, in
This even works if your neural net is part of a sklearn pipeline, in

This comment has been minimized.

Copy link
@benjamin-work

benjamin-work Nov 23, 2018

Author Collaborator

I pronounce it "ess-kay-learn", therefore I would use "an". Or do most people "translate" it to "scy-kit-learn"?


Almost all arguments should work out of the box. Therefore, you get
command line arguments for the number of epochs, learning rate, batch
size, etc. for free. Morevoer, you can access the module paremeters

This comment has been minimized.

Copy link
@taketwo

taketwo Nov 22, 2018

Contributor

Same here. I wonder if this documentation duplication can be avoided somehow?

This comment has been minimized.

Copy link
@benjamin-work

benjamin-work Nov 23, 2018

Author Collaborator

Yes, I wondered about that too. I wanted to avoid sending readers this way and that way to collect all relevant information, hence the duplication. Any ideas?

@benjamin-work

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 23, 2018

Thanks for the feedback.

it helps me organize my runs directory, automatically creating directories/filenames for different experiments based on the parameters

This sounds like something that could be combined with the current feature. Do you think that would be a good idea?

Add the option to have custom defaults.
E.g., if you would like to use batch_size=256 as a default instead of
128, you can now pass a dict `{'batch_size': 256}` to
`parse_args`. This will not only update your model to use those
defaults but also change the help to show your custom defaults.

To achieve the latter effect, it was necessary to parse the sklearn
docstrings for default values and replace them by the new
default. This turned out to be more difficult than expected because
the docstring defaults are not always written in the same fashion. I
tried to catch some variants that I found but there are certainly more
variants out there. It should, however, work fine with the way we
write docstrings in skorch.
@benjamin-work

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 27, 2018

@ottonemo I added a feature that allows to change the defaults (see commit message). Please review again.

@ottonemo
Copy link
Collaborator

left a comment

LGTM, some comments though

Show resolved Hide resolved docs/user/helper.rst Outdated
Show resolved Hide resolved examples/cli/README.md
Show resolved Hide resolved skorch/cli.py

def print_help_and_exit(estimator):
print_help(estimator, defaults=defaults)
sys.exit()

This comment has been minimized.

Copy link
@ottonemo

ottonemo Dec 10, 2018

Collaborator

Maybe use exit(1) to indicate non-normal termination?

This comment has been minimized.

Copy link
@benjamin-work

benjamin-work Dec 11, 2018

Author Collaborator

But should the error code not be 0 for a normal help option?

This comment has been minimized.

Copy link
@ottonemo

ottonemo Dec 11, 2018

Collaborator

Well, not sure, the usual tools don't seem to agree on that issue:

$ cat -h 2>/dev/null; echo $?
1
$ sed -h 2>/dev/null; echo $?
1
$ dd -h 2>/dev/null; echo $?
1
$ grep --help >/dev/null; echo $?
0
$ make -h >/dev/null; echo $?
0
$ bzcat -h; echo $?
0
$ bash --help; echo $?
0

But it might be a good idea to exit with an error code for cases when deploying the script to distributed/automated training. An illegal option might be detected sooner if the script fails to execute due to an unknown option or a left-over -h.

ottonemo and others added some commits Dec 11, 2018

Fix typo in docs/user/helper.rst
Co-Authored-By: benjamin-work <benjamin.bossan@ottogroup.com>
@benjamin-work

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 11, 2018

@ottonemo pls review again

@ottonemo ottonemo merged commit 3712369 into master Dec 13, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@ottonemo ottonemo deleted the feature/quick-clis branch Dec 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.