Average pooling with padding - behavior on edges

[oscarriddle]

Hi,

I'm currently working on converting a TensorFlow trained InceptionV3 model to Caffe model, because I want to test the inference performance under Caffe. I've almost got there, during which several interesting issues are encountered so here is the one.

The average pooling layer calculates different output compared to TensorFlow, when padding is enabled.

Observation:
If the padded elements are contained within current pooling kernel, caffe will compute the average of all the inputs, including the padded values(which are "0"s). Hence its result gets smaller and severely affected by those invalid "0"s.

Root-Cause:
The root-cause is that no matter whether the padding exists in current computing kernel or not, the pooling_size is fixed to the parameter "kernel_size", like 9 for 3x3 kernel.

Here is part of debugging output within every iteration in AVE pooling:
(Input_size 35x35, kernel_size 3x3, padding 1)
pooled_height_:35, pooled_width_:35 stride_h_:1, stride_w_:1 pad_h_:1, pad_h_:1 height_:35, width_:35 hstart:0, hend:2 wstart0, wend:2 pool_size9 <-Here-> top_data0.730075 pooled_height_:35, pooled_width_:35 stride_h_:1, stride_w_:1 pad_h_:1, pad_h_:1 height_:35, width_:35 hstart:0, hend:2 wstart0, wend:3 pool_size9 <-Here-> top_data0.913562 pooled_height_:35, pooled_width_:35 stride_h_:1, stride_w_:1 pad_h_:1, pad_h_:1 height_:35, width_:35 hstart:0, hend:2 wstart1, wend:4 pool_size9 <-Here-> top_data0.615629 pooled_height_:35, pooled_width_:35 stride_h_:1, stride_w_:1 pad_h_:1, pad_h_:1 height_:35, width_:35 hstart:0, hend:2 wstart2, wend:5 pool_size9 top_data0.370382

Note that the hstart, hend, wstart, wend are updating every iteration, but pool_size stays at 9.

Analysis:
Generally speaking, average pooling shan't takes invalid padded values (like 0), because it will pollute the feature map and downgrade the significance of the edge elements.

So I dive into the pooling_layer.cpp, and find below codes for calculating the variable "pool_size".

case PoolingParameter_PoolMethod_AVE:
    for (int i = 0; i < top_count; ++i) {
      top_data[i] = 0;
    }
    // The main loop
    for (int n = 0; n < bottom[0]->num(); ++n) {
     for (int c = 0; c < channels_; ++c) {
        for (int ph = 0; ph < pooled_height_; ++ph) {
          for (int pw = 0; pw < pooled_width_; ++pw) {
            int hstart = ph * stride_h_ - pad_h_;
            int wstart = pw * stride_w_ - pad_w_;
            int hend = min(hstart + kernel_h_, height_ + pad_h_);
            int wend = min(wstart + kernel_w_, width_ + pad_w_);
            int pool_size = (hend - hstart) * (wend - wstart);  //<-Here->
            hstart = max(hstart, 0);
            wstart = max(wstart, 0);
            hend = min(hend, height_);
            wend = min(wend, width_);
            for (int h = hstart; h < hend; ++h) {
              for (int w = wstart; w < wend; ++w) {
                top_data[ph * pooled_width_ + pw] +=
                    bottom_data[h * width_ + w];
              }
            }
            top_data[ph * pooled_width_ + pw] /= pool_size; //<-Here->
          }
        }
        // compute offset
        bottom_data += bottom[0]->offset(0, 1);
        top_data += top[0]->offset(0, 1);
      }
    }
    break;

Obviously, the variable "pool_size" determines the valid number of elements in each iteration. I can get the designer's idea is to update the pool_size along with kernel sliding but not fix it to a value. However, it's been initialized before the "hstart, hend, wstart, wend" getting updated. So the pool_size couldn't be updated eventually.

Solution:
My modification is quite simple, but I think it's very important.

Hope to hear some comment from your side.

Thanks,
Xiaolun Cao

[Noiredd]

Copy of @oscarriddle's comment in another thread:

I tried to compare the output values of average pooling layer between Caffe and TensorFlow, and find Caffe's AVE pooling result is always smaller. From the source code, I located the issue that the variable "pool_size", which is the denominator of averaging, is fixed to the kernel size (like 9 for 3x3).
Hence if the pooling kernel overlaps some padded elements (for instance, 0), the averaged value becomes smaller. While I think the correct pooling method shall only calculate those valid elements.
This is a quite simple issue but also significant. Because if someone's network implements many average pooling layers within convolutions, the averaged outputs always lost the feature map information which is downgrading the significance of those elements around the edges.

I will take a closer look at this soon. In the meantime, please review your code again - there are several tests that this change fails to pass.

[oscarriddle]

Hi,

I checked the Travis CI failed log. The compilation is successfully done but failed at the running test of AVE pooling layer, looks like the output values aren't equal to expected values.

Failed log:

[  FAILED  ] 4 tests, listed below:
[  FAILED  ] LRNLayerTest/0.TestForwardWithinChannel, where TypeParam = caffe::CPUDevice<float>
[  FAILED  ] LRNLayerTest/1.TestForwardWithinChannel, where TypeParam = caffe::CPUDevice<double>
[  FAILED  ] PoolingLayerTest/0.TestForwardAve, where TypeParam = caffe::CPUDevice<float>
[  FAILED  ] PoolingLayerTest/1.TestForwardAve, where TypeParam = caffe::CPUDevice<double>

Besides, not quite sure what happened to LRNLayer test.
Could you help take a closer check on this? If I guess right, the tests of relevant layers also need to be redesigned.

Thanks,
Xiaolun Cao

[oscarriddle]

Hi,

Depending on this fix, I successfully converted a TensorFlow trained InceptionV3 model to a caffe model, and get the exact same final inference prediction result compared to the the TensorFlow.

Now I believe that TensorFlow model can definitely be converted to Caffe with almost no accuracy sacrifice (loss around 0.0001%). At the meantime, InceptionV3 network can also runs on Caffe, too! I did it.

Next step, I will try to evaluate the inference performance difference between the TF and Caffe.

Thanks,
Xiaolun Cao

[Noiredd]

The LRN layer creates some layers internally, among them the PoolingLayer; so if we break one, the other fails too. Looks like you're right: the relevant tests would have to be redesigned as well - not only for the Pooling, but LRN layer as well.

At this point the question arises, whether such a change doesn't break existing models. Maybe this should come as an optional behavior, with a boolean switch defined in caffe.proto, to allow the users to choose the layer's behavior at the edges? See #6282 for the same idea applied to a different problem.

PS: this PR's commit history became quite weird: looks like you're trying to merge back some 200 commits that are already in master.

[oscarriddle]

OK, that makes sense.

I'm not quite familiar with Pull Request, I tried git pull on this branch so those existed commits are somehow merged into this branch. Maybe I should start over from a clean branch.

That's a question indeed, an optional toggle seems good for now. Let me have a try on my local repo, and let's have a talk again when everything goes smoothly.

Thanks,
Xiaolun Cao

[Noiredd]

Closing, we'll continue in #6303.