Give phase to Net

[shelhamer]

After #1794 since dump_network is dropped for incompatibility here.

Make Net master of phase as is its proper due and let layers inherit its phase. This fixes #1430 and satisfies the phase part of #1500. Here's to taking a small step toward abolishing the singleton.

• phase is set at instantiation and should never change.
• phase defaults to TEST by the caffe.proto default. This fits the wishes in Should we set the phase as TEST when creating the test_net in solver.cpp? #192, but should there be a default phase at all?

Misgivings:

• the explicit Net(const string& param_file, Phase phase) constructor works, but what of stage and level and further extensions to net state? Should the parameter file constructor be done away with and all these set through NetParameter? The caffe tool and wrappers will need to learn about NetState.
• exposing the Phase enum seemed like a hassle, so 001f060 just exposes the proto values as caffe.{TRAIN,TEST} for instantiating caffe.Net. I suppose this could have a pytest.

[jeffdonahue]

Maybe it would make sense to add phase to TransformationParameter, setting the phase from the owning net by default? (Keeps the single-argument constructor, and allows one to override defaults, e.g., if one wanted to randomly crop/mirror at test time for some reason.)

[shelhamer]

@jeffdonahue @longjon what if phase is brought down into the layers that need it? These layers would have phase added to their parameter message and it would default to the Net's during Net::Init(). This marks which layers actually care about phase and does away with the need for the Net pointer to sidestep needing to make a dummy Net for all the tests, which is pretty annoying as-is.

[jeffdonahue]

Sounds fine, but seems like you might as well just add it to Layer itself and have the Net set it for all Layers (rather than worrying about whether they need it).

[shelhamer]

@longjon @jeffdonahue I think this could work -- let me know what you think.

[longjon]

My thoughts on this:

Phase is just a net-level parameter. It would be nice if phase could be ignored for nets that don't have layers that depend on phase, but it's also important that phase be specified explicitly for nets that do depend on it, since test/train confusion is very bad.

So I'm okay with the way this is done now as an incremental step, because per-layer phase-dependence isn't already explicitly noted, and doing so is a pain. But I think I would actually prefer if phase-dependent layers took phase explicitly. That way the user who just wants to compute doesn't have to think about this irrelevant phase thing, while at the same time the user who runs a train/val net must explicitly and immutably choose a phase, at the threat of construction-time failure (I am both users, at times).

[jeffdonahue]

This LGTM. I don't have much of an opinion on the issues @longjon raised.

[shelhamer]

Note to self: all this needs is new pyreformation constructors to cast phase.

[shelhamer]

This is reconciled with pyreformation #1703 and now caffe.Nets are instantiated with phase as caffe.{TRAIN,TEST}.

• I'm content with exposing the enum as done here -- while there is a boost::python enum sharing protobuf's enum in this way will lead to gross identifiers and type woes.
• The phase arg should be converted to a keyword argument in the Net constructors, and so with level, stage, and eventually mode and device too.

A follow-up PR will further change the pycaffe interface to yield to Net all that is Net's.

[lukeyeager]

@shelhamer you ended up creating a Python interface with this PR which isn't actually a "valid" Python interface. Boost.Python lets you do some cool stuff with pattern matching like allowing these two function signatures:

net1 = Net('deploy.prototxt', caffe.TEST)
net2 = Net('deploy.prototxt', 'weights.caffemodel', caffe.TEST)

But you can't actually do that with pure Python:

>>> class Net():
...     def __init__(self, network, weights, phase):
...         pass
...     def __init__(self, network, phase):
...         pass
... 
>>> net1 = Net('deploy.prototxt', 1)
>>> net2 = Net('deploy.prototxt', 'weights.caffemodel', 1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __init__() takes exactly 3 arguments (4 given)

The second definition of __init__ overrides the first.

I wish this would have been done with named keyword arguments instead, like:

class Net():
    def __init__(self, network, phase=0, weights=None):
        pass

net1 = Net('deploy.prototxt', phase=1)
net2 = Net('deploy.prototxt', phase=1, weights='weights.caffemodel')

Actually, if you would have just put "phase" as the second positional argument for both signatures, we could have salvaged it because you can provide keyword arguments as positional arguments if they line up with the order in which you specified the keyword arguments. As it is, we can't change it now without breaking code. 😞

NOTE - I'm pointing this out because my upcoming PR adding stage and level as optional arguments will either not be backwards compatible, or will include duplicated code.

[shelhamer]

@lukeyeager right, thanks for the clear exposition. I would have liked keyword args too and I agree it makes for a clearer interface. I'm traveling now but will look at #3863.