Refactor Solver to allow interactive stepping

[longjon]

In this PR, the main loop of Solver::Solve is factored out into Solver::Step, which is exposed to Python. This allows interactive stepping of the solver.

Warning: I've only actually tested a parallel version of this on another branch.

[rodrigob]

@longjon I am currently playing on top of this branch.
How do you handle things that happen at the end of Solver:Solve on python side ?
E.g. things like handling snapshot_after_train().

Right now I am simply planning to expose these to python, so as to have a python re-implementation of Solver::Solve, and then add there callbacks to python plotting magic (see #481).

[longjon]

@rodrigob Cool, let me know (or PR this branch) if you find any errors.

In my use, I don't worry about things happening at the end of Solve, because there is no end; I am always running things until I've decided they've converged, diverged, or otherwise failed.

But yes, I essentially use a Python implementation of Solve, which is just a loop around solver.step, with any extras that I want thrown in, and preceded by a restore (not exposed yet) or a copy_from (#1195) if desired. Exposing Snapshot as well seems right.

[jeffdonahue]

LGTM

[muupan]

I'm using this PR for implementing online reinforcement learning. I think it's very useful but needs a small fix to work safely:

Solver's member variable initialized_ should be initialized with false in the constructor, otherwise calling Step() will cause undefined behavior because it checks the value of the uninitialized variable initialized_ at https://github.com/longjon/caffe/blob/solver-step/src/caffe/solver.cpp#L191

This is how I fixed it: muupan@d098036

[longjon]

@muupan you're right, of course. I've updated this with a rebase of my most recent working version.

[longjon]

Updated: the snapshot logic has been changed to properly snapshot after every _k_th iteration, instead of snapshotting before the iteration following every _k_th. (In other words, snapshotting has been moved to the end of the solver loop.)

This simplifies the logic (except the "snapshot after train" logic), and ensures that Step(k) will end up with a snapshot on disk, instead of waiting for the next Step call, which can be important if something goes wrong between those two calls.

[jeffdonahue]

IMO since this method is common setup specific to the Solver class itself, it should just be given a new name like Initialize() (and then called separately before PreSolve() @ line 236), so that anyone subclassing Solver doesn't need to remember to call it.

[longjon]

I agree that forcing subclassers to call PreSolve is awkward. Is there any reason we don't just call PreSolve from Init? That would simplify the logic and get rid of initialized_, as there would be no such thing as an uninitialized solver, and then it makes sense to do the base class initialization right in Init. I think I'll try it and update the PR.

[jeffdonahue]

Sounds good to me!

[longjon]

Actually, we can't do that, because virtual functions can't be called from constructors. Here's a slightly more radical idea, which I've implemented now: get rid of Solver::PreSolve and just use the constructors, since constructors now provide the same functionality. SGDSolver::PreSolve, which was the only actual PreSolve, remains as it was and gets called from the SGDSolver constructor.

[jeffdonahue]

can be removed since you changed the call to ForwardPrefilled?

[longjon]

The changed call is in Solve, this is in Step, where the call is to ForwardBackward. An alternative is to remove bottom_vec but call ForwardPrefilled and Backward separately; that's what I've now implemented.

[longjon]

Actually, I think I'll revert that, because it makes #1663 awkward. We should probably just clean up the Net interface to avoid these dummy vectors at some later point.

[longjon]

@jeffdonahue I think I'm satisfied with this now; merge when ready.

[jeffdonahue]

Cool, thanks Jon