msracver / Deformable-ConvNets

Deformable Convolutional Networks
MIT License
4.02k stars 953 forks source link

this gemm operation seemes useless #245

Closed BIGKnight closed 5 years ago

BIGKnight commented 5 years ago

Hi, i have been reading your implementation line by line. And I found a code block really strange. In the file "master/DCNv2_op/modulated_deformable_convolution-inl.h", and in the backward function. and the code is here: https://github.com/msracver/Deformable-ConvNets/blob/81d3bd03239aef35030101e02ffe74b21c1e2a25/DCNv2_op/modulated_deformable_convolution-inl.h#L230-L271 the first linalg_gemm calculates the MxN matrix and writes it into the col_buffer_3d, it seemes to calcalute the input_feature_matrix for calculating the gradient of the kernel, because the gradients of the kernel are calculated by input_feature_matrix.T * (dloss / doutput_grad). However, the gemm's result was not right, because you use the out_grad_3d instead of in_data. And in fact, you also calculate the true input_feature_matrix by calling the function "modulated_deformable_im2col".And this one will override the result of the first gemm. so as a result, although the result of the backward process was not affected, the code https://github.com/msracver/Deformable-ConvNets/blob/81d3bd03239aef35030101e02ffe74b21c1e2a25/DCNv2_op/modulated_deformable_convolution-inl.h#L232-L236 can be removed. I don't ensure that my viewpoint was right, maybe I missed some essential part. hope for your reply, thanks a lot. And thanks again for your team's great work.it's awesome.

BIGKnight commented 5 years ago

I found my missed part.I'm sorry for my ignorance.