Python: setting phase to test before creating Net

[ssafar]

Using the Python wrapper (on a fairly recent dev branch), I noticed that even though I called Net.set_phase_test() on my Net object after creating it, it was still returning training results. To get the test ones, I needed to recreate the network another time.

After some investigation, it looks like that in the C++ API, phase is not a property of the Net object but one of a singleton object Caffe. In retrospect, it's not surprising: in order to create the Net, we need to know beforehand whether we'd like to see the test or the training version.

The main problem seems to be that we can only call set_phase()_ after having created a Net (unlike the C++ API, where we have access to the singleton even before any Nets get created).

Steps to reproduce:

• create a caffe.Net object, loading a model that is different for the train & test phases (e.g. different inputs, as in the example LeNet model)
• call set_phase_test() on the net
• forward()
• look at blobs in the net

(Somewhat naively) expected behavior:

• blobs contain test data / results
• the network architecture corresponds to the test phase ("we just made it so using set_phase_test()?")

Actual behavior:

• blobs contain training data / results
• the network architecture corresponds to the training phase

Workaround (I haven't found a simpler one yet; is there any?):

• create a Net object with any model
• call set_phase_test() on it
• delete this Net object and create the one we wanted to create in the first place

As a proposed solution, the set_phase_* methods could be moved to somewhere else, just like in C++, both to make the expected order of calls (first set phase, then create net) more obvious and to make it possible to set phase before creating Nets. Or... what do you think?

[longjon]

Yep, this issue has been on my mind lately; thanks for the very cogent writeup. This is dangerous behavior.

I think we've avoided exposing set_phase_* outside Net due to a general consensus that phase should really be a per-Net property (even in Caffe proper). It would be a bit cumbersome to deprecate the Net methods, only to un-deprecate them later. However, given the current state of affairs, that's probably what we should do: expose the singleton methods at top level in the caffe module, and produce warnings (at least) for the Net level ones.

Or maybe we don't need the concept of phase switching anymore, and we can make phase a creation-time property of Nets? Thoughts @shelhamer @jeffdonahue et al.?

[shelhamer]

I think we've avoided exposing set_phase_* outside Net due to a general consensus that phase should really be a per-Net property (even in Caffe proper)

This is exactly why. In our copious free time we've been wishing to make phase a property of Net throughout the library and drop it from the singleton since it leads to confusion and danger.

Or maybe we don't need the concept of phase switching anymore, and we can make phase a creation-time property of Nets?

It's been a rare occasion that I've switched phase, but it has come up and it's not so difficult to leave that option open.

p.s. #192 is related in that whatever we do it's worth considering the default of phase while we make the change.

[longjon]

@shelhamer Switching phase still feels a little wrong to me, at least the way it currently interacts with consolidated nets. To me, the phase should be just an extra parameter in the function defined by the net, but as it is that parameter has a kind of hysteresis, because changing the phase after net construction isn't the same as setting the phase before construction. So I'm curious as to what your use case is... let's chat about it sometime.

[shelhamer]

@longjon actually now that I review it I think it was an artifact of how
Caffe did backward at the time and the usage was a far corner case. We
should make it a construction parameter and leave it at that, so that the
phase-as-parameter view of Forward and Backward holds.

On Friday, October 10, 2014, longjon notifications@github.com wrote:

@shelhamer https://github.com/shelhamer Switching phase still feels a
little wrong to me, at least the way it currently interacts with
consolidated nets. To me, the phase should be just an extra parameter in
the function defined by the net, but as it is that parameter has a kind of
hysteresis, because changing the phase after net construction isn't the
same as setting the phase before construction. So I'm curious as to what
your use case is... let's chat about it sometime.

—
Reply to this email directly or view it on GitHub
#1250 (comment).

[jeffdonahue]

We could just have Init use the setting given by NetParameter rather than
look at caffe::phase, seems redundant to add a new ctor/init param.
On Oct 10, 2014 8:36 AM, "Evan Shelhamer" notifications@github.com wrote:

@longjon actually now that I review it I think it was an artifact of how
Caffe did backward at the time and the usage was a far corner case. We
should make it a construction parameter and leave it at that, so that the
phase-as-parameter view of Forward and Backward holds.

On Friday, October 10, 2014, longjon notifications@github.com wrote:

@shelhamer https://github.com/shelhamer Switching phase still feels a
little wrong to me, at least the way it currently interacts with
consolidated nets. To me, the phase should be just an extra parameter in
the function defined by the net, but as it is that parameter has a kind
of
hysteresis, because changing the phase after net construction isn't the
same as setting the phase before construction. So I'm curious as to what
your use case is... let's chat about it sometime.

—
Reply to this email directly or view it on GitHub
#1250 (comment).

—
Reply to this email directly or view it on GitHub
#1250 (comment).

[shelhamer]

@jeffdonahue right -- I agree about how to set the phase field but the
point here is that the Net will need a member var for this and all the
layers will need to check that instead of the singleton.

On Fri, Oct 10, 2014 at 9:07 AM, Jeff Donahue notifications@github.com
wrote:

We could just have Init use the setting given by NetParameter rather than
look at caffe::phase, seems redundant to add a new ctor/init param.
On Oct 10, 2014 8:36 AM, "Evan Shelhamer" notifications@github.com
wrote:

@longjon actually now that I review it I think it was an artifact of how
Caffe did backward at the time and the usage was a far corner case. We
should make it a construction parameter and leave it at that, so that
the
phase-as-parameter view of Forward and Backward holds.

On Friday, October 10, 2014, longjon notifications@github.com wrote:

@shelhamer https://github.com/shelhamer Switching phase still feels
a
little wrong to me, at least the way it currently interacts with
consolidated nets. To me, the phase should be just an extra parameter
in
the function defined by the net, but as it is that parameter has a
kind
of
hysteresis, because changing the phase after net construction isn't
the
same as setting the phase before construction. So I'm curious as to
what
your use case is... let's chat about it sometime.

—
Reply to this email directly or view it on GitHub
#1250 (comment).

—
Reply to this email directly or view it on GitHub
#1250 (comment).

—
Reply to this email directly or view it on GitHub
#1250 (comment).

[ronghanghu]

After making a mistake yesterday, I also feel it is wrong to switch phase after creating the net. In yesterday's experiments I defined different network structure for TRAIN and TEST (by using different include in network prototxt). When I tried to switch phase from TRAIN to TEST by calling net.set_phase_test() I mistakenly thought pycaffe will switch my network to the TEST net structure defined in my prototxt, and later after getting very weird results I realized that I misunderstood it and actually pycaffe made no changes on network structure.

[shelhamer]

The interface issue is addressed by #1728 but Net should still own phase and it should be set at initialization and not altered #1500.