MVNLayer across_channels is broken

[cdoersch]

In MVNLayer's forward, there's lots of cases where there's a gemv between a num-by-dim matrix and sum_multiplier_. When across_channels is true, dim will be channels*width*height. Unfortunately for the gemv, sum_multiplier_ is only width*height, since it's created with

   sum_multiplier_.Reshape(1, 1,
      bottom[0]->height(), bottom[0]->width());
    Dtype* multiplier_data = sum_multiplier_.mutable_cpu_data();
    caffe_set(sum_multiplier_.count(), Dtype(1), multiplier_data);

Hence, the gemv multiplies the matrix by random garbage whenever this flag is active.

The fix is just to check for across_channels during init and, if it's true, initialize sum_multiplier to be channels*width*height With this fix at least forward() seems to work properly. Since I'm guessing that the layer hasn't been extensively tested with this flag enabled, somebody may want to test backward() too...

[seanbell]

I think #1979 addresses this bug, but you should confirm this. I don't think anyone from BVLC is reviewing that PR yet.

[cdoersch]

#1979 does address this bug, but also does a lot of other things; probably too much for a single PR. For instance, it has a new file called mvn_blob_helper.cpp which seems like a new, MVN-layer specific abstraction layer; I think there needs to be an very strong reason why such a file needs to exist in caffe, and in five minutes I couldn't figure out why it needs to be there. This is probably why nobody in BVLC is reviewing it.

Overall, it's not obvious that this PR will be merged anytime soon, but this is a major bug that can be fixed with a 3-line change.

[jyegerlehner]

it has a new file called mvn_blob_helper.cpp ...
and in five minutes I couldn't figure out why it needs to be there.

mvn_blob_helper.cpp is used by InverseMVNLayer can figure out which of its bottom blobs is the mean and which is the variance (and by MVNLayer to do the same for its top blobs). Prior to adding it (and blob_finder.hpp), layers have no way of finding blobs by name, as far as I was able to find.

I discovered the across_channels bug (and a couple others) in the course of implementing the tests for InverseMVNLayer. These defects had to be fixed in order for InverseMVNLayer tests to be completed and bring them to a passing state. So I fixed them. I didn't mean any harm by it.

I think there needs to be an very strong reason why such a file needs to exist in caffe,

I disagree that a file is such a liability that it needs extraordinary justification. I think designs that consist of many simple collaborating classes are superior to designs that have few large and complicated classes. Such designs utilizing simpler classes will inevitably have more files.

[shelhamer]

Fixed by #2964