I personally don't like the idea of having to use argparse instead of python-gflags, which is more flexible and allows one to define flags in multiple modules, and has a C++ counterpart (honestly, I may be biased since I like gflags), thus this change seems unnecessary. Maybe change the code in script/resize_and_crop_images.py to use gflags.

@sergeyk @shelhamer Any comments on the choice of commandline flag support?

My two cents:

Python has standard libraries, without a clear reason not to use them, pythonistas will expect to see standard libraries used. "Because it looks more like C++" will not fly for pythonistas.

At the end of the day it is about "do you want people to be able to use caffe only from python or not". If the answer is yes, then this change make sense. If the answer is "no this is C++ code, with some python tools", then gflags is fine (since echoes with the C++ part of the codebase).

(1) python-gflags is pure-python-implemented.

(2) python-gflags is fully open-source.

(3) caffe is never designed to run "purely on python". It is a C++ library. Decaf serves (more or less) a library under python.

(4) Yes, caffe is C++ code with some python tools.

Yangqing

On Thu, Feb 13, 2014 at 10:04 AM, Rodrigo Benenson <[email protected]

wrote:

My two cents:

Python has standard libraries, without a clear reason not to use them,
pythonistas will expect to see standard libraries used. "Because it looks
more like C++" will not fly for pythonistas.

At the end of the day it is about "do you want people to be able to use
caffe only from python or not". If the answer is yes, then this change make
sense. If the answer is "no this is C++ code, with some python tools", then
gflags is fine (since echoes with the C++ part of the codebase).

Reply to this email directly or view it on GitHubhttps://github.com//pull/107#issuecomment-35006911
.

I am not particularly fond of the "always use standard libraries" idea. Scientific computation with python will never take off if we stick with standard libraries in the first place (matrices could be a list of list of floats, vector inner products will pythonically be sum(x*y for x, y in zip(vx, vy)), but we all know what follows). This being said, I think a little more explanation why argparse may be bad is in order:

The thing I do not like argparse is that you will need to maintain a parser instance under which flags are defined. This makes it difficult when multiple modules define their own flags, and when assembled together form a binary having all these flags naturally combined.
I started using optparse and then argparse in Yangqing/mincepie but was constantly bugged by that. The use case in mincepie is that, you will need to define mapreduce related flags in one python file and launcher related flags in another python file - argparse provides little mechanism to elegantly work with that. In contrast, gflags maintains a sort-of global (cough cough) gflags.FLAGS variable that holds the flags defined everywhere. It also supports an easier way to look at the flags when there are a lot of them (which will be true in any non-trivial system). Take the mincepie mapreduce program for example:

python mincepie/demo/wordcount.py --help
[a bunch of things omitted]
flags:

mincepie.launcher:
-?,--[no]help: show this help
--[no]helpshort: show usage only for this module
--[no]helpxml: like --help, but generates XML output
--launch: The launch mode. See mincepie.launcher.launch() for details.
(default: 'local')
--loglevel: The level for logging. 20 for INFO and 10 for DEBUG.
(default: '20')
(an integer)
--num_clients: The number of clients. Does not apply in the case of MPI.
(default: '1')
(an integer)
--sbatch_args: The sbatch arguments;
repeat this option to specify a list of values
(default: '[]')
--sbatch_bin: The command to call sbatch
(default: 'sbatch')
--scancel_bin: The command to call scancel
(default: 'scancel')
--slurm_python_bin: The command to call python
(default: 'python')
--slurm_shebang: The shebang of the slurm batch script
(default: '#!/bin/bash')

mincepie.mapreducer:
--input: The input pattern.
(default: '')
--mapper: The mapper class for the mapreduce task
(default: '')
--output: The string passed to the writer
(default: '')
--reader: The reader class for the mapreduce task
(default: '')
--reducer: The reducer class for the mapreduce task
(default: '')
--writer: The reader class for the mapreduce task
(default: '')

mincepie.mince:
--address: The address of the server
(default: '127.0.0.1')
--password: The password for server client authentication
(default: 'default')
--port: The port number for the mapreduce task
(default: '11235')
(an integer)
--report_interval: The interval between which we report the elapsed time of mapping
(default: '10')
(an integer)
--timeout: The number of seconds before a client stops reconnecting
(default: '60')
(an integer)

gflags:
--flagfile: Insert flag definitions from the given file into the command line.
(default: '')
--undefok: comma-separated list of flag names that it is okay to specify on the command line even if the
program does not define a flag with that name. IMPORTANT: flags in this list that have arguments MUST
use the --flag=value format.
(default: '')

Now the reasons are clear @Yangqing :)

I raised the issue as a matter of consistency. Since gflags is necessary to compile anyway, python-gflags is a simple (and documented) dependency.

@sergeyk if you feel the same, I'll revert the conversion to argparse in detector.py (but of course keep all the other changes!)

I'll take care of making requirements.txt a little more flexible.

Plus, I think it is potentially a trap to have separate requirements.txt for the wrapper and the scripts and we should perhaps keep a single one.

@Yangqing
I like that you can define flags in separate modules and import them easily with python-gflags. I haven't used that feature, and in fact had to code something like that up myself for argparse in another repo (vislab).

That said, python-gflags looks un-pythonic and unmaintained to me. @shelhamer Although you can close the issue to drop the py-gflags dependency, I don't see a need to revert the detector.py changes right now. If we ever have to share flags defined in that module with another module, then we can go back to gflags -- or find a better way.

I agree. Having flag definitions we are used to in individual demo codes
should be fine, and we can (for now) use python-gflags as the standard in
codes that may be used in a large scale.

python-gflags is quite un-pythonic but probably won't be buggy given the
heavy usage test at google.

Yangqing

On Thu, Feb 13, 2014 at 1:30 PM, Sergey Karayev [email protected]:

@Yangqing https://github.com/Yangqing
I like that you can define flags in separate modules and import them
easily with python-gflags. I haven't used that feature, and in fact had to
code something like that up myself for argparse in another repo (vislab).

That said, python-gflags looks un-pythonic and unmaintained to me.
@shelhamer https://github.com/shelhamer Although you can close the
issue to drop the py-gflags dependency, I don't see a need to revert the
detector.py changes right now. If we ever have to share flags defined in
that module with another module, then we can go back to gflags -- or find a
better way.

Reply to this email directly or view it on GitHubhttps://github.com//pull/107#issuecomment-35028293
.

Hi,

I've been lurking for a bit, but haven't had the time to contribute much in the way of discussion; talked to Yangqing and Evan about eigen-boost branch a while ago.

Maybe it's too much effort for too little pay off at this point, but docopt could be a good solution. It's familiar (at least the cli syntax), pythonic, and could help enforce documentation of command line options. You write out your help message in a formal syntax and it automatically parses everything for you.

http://docopt.org/

Michael

p.s. @kloudkl I loosened up the requirements.txt, included @longjon 's patch, and added the shebangs.