BVLC / caffe

Caffe: a fast open framework for deep learning.
http://caffe.berkeleyvision.org/
Other
34.01k stars 18.71k forks source link

A bug in pooling_layer.cpp? #6881

Open freeniliang opened 4 years ago

freeniliang commented 4 years ago

@csukuangfj @Noiredd

Issue summary

A bug in pooling_layer.cpp?

original code:

if (padh || padw) { // If we have padding, ensure that the last pooling starts strictly // inside the image (instead of at the padding); otherwise clip the last.` if ((pooledheight - 1) strideh >= height_ + padh) { --pooledheight; } if ((pooledwidth - 1) stridew >= width_ + padw) { --pooledwidth; } CHECK_LT((pooledheight - 1) strideh, height_ + padh); CHECK_LT((pooledwidth - 1) stridew, width_ + padw); } I think that “if (padh || padw) ” should be removed!

For example: Input feature map = hiwi= 66,kernel=1,stride=2,pad=0,round_mode=CEIL; ho=ceil_div((6+20-1) ,2)+1=4 (ho-1)stride=(4-1)*2=6=hi+pad=6+0 In this case, the 4th output point is out side of the input map, ho/wo must be decreased by 1!

So, “if (padh || padw) ” should be removed!

csukuangfj commented 4 years ago

https://github.com/BVLC/caffe/blob/04ab089db018a292ae48d51732dd6c66766b36b6/src/caffe/layers/pooling_layer.cpp#L107-L118

I think that “if (padh || padw) ” should be removed!

You'll fail at line 116 https://github.com/BVLC/caffe/blob/04ab089db018a292ae48d51732dd6c66766b36b6/src/caffe/layers/pooling_layer.cpp#L116

In this case, the 4th output point is out side of the input map, ho/wo must be decreased by 1!

Pleas read the code further: https://github.com/BVLC/caffe/blob/04ab089db018a292ae48d51732dd6c66766b36b6/src/caffe/layers/pooling_layer.cpp#L168-L175

At line 175, h < hend will be false and the for loop is not executed.

freeniliang commented 4 years ago

https://github.com/BVLC/caffe/blob/04ab089db018a292ae48d51732dd6c66766b36b6/src/caffe/layers/pooling_layer.cpp#L107-L118

I think that “if (padh || padw) ” should be removed!

You'll fail at line 116 https://github.com/BVLC/caffe/blob/04ab089db018a292ae48d51732dd6c66766b36b6/src/caffe/layers/pooling_layer.cpp#L116

In this case, the 4th output point is out side of the input map, ho/wo must be decreased by 1!

Pleas read the code further: https://github.com/BVLC/caffe/blob/04ab089db018a292ae48d51732dd6c66766b36b6/src/caffe/layers/pooling_layer.cpp#L168-L175

At line 175, h < hend will be false and the for loop is not executed.


“if (padh || padw) ” should be removed!You'll fail at line 116 -----I think that line 116 will not fail, because pooledheight/pooledheight is already decreased by 1, so, (pooledheight - 1) * strideh must be less than height_ + padh at line 116;

At line 175, h < hend will be false and the for loop is not executed. ----- Yes, this loop will not be executed, but the number of output has been identified in the following below:https://github.com/BVLC/caffe/blob/04ab089db018a292ae48d51732dd6c66766b36b6/src/caffe/layers/pooling_layer.cpp#L107-L118

In this case, you will find that the data at the boundary(the most right/bottom) are invalid(AVE:0.0; MAX:-FLT_MAX). In order to drop the invalid data, a slice layer must be added behind the pool layer, but if “if (padh || padw) ” is removed, the output of pool layer can be used by next layer without inserting slice layer!