[GitHub] marcoabreu edited a comment on issue #13955: adding the gluon implementation of deformable convolution

[GitBox <gi...@apache.org>]

marcoabreu edited a comment on issue #13955: adding the gluon implementation of deformable convolution
URL: https://github.com/apache/incubator-mxnet/pull/13955#issuecomment-456708201
 
 
   I see, thanks for elaborating :) You just added the new gluon implementation layer, so you don't have to verify the functional correctness of the underlying operators since they are already tested elsewhere as you stated, but what do you think of testing the layer you just added? 
   
   Within the code you are making a few assumptions regarding the parameters of the operators, shapes and channels that might change in future. With a few unit tests you could assert that the assumptions you are making are still valid in future if somebody makes changes in the underlying code. 
   
   Testing the constructor as well as a few simple calls that trigger the different branches of ```hybrid_forward``` and validate the results should be sufficient. I'm currently working on branch-testcoverage that would show you these kind of cases, but until then, we have to take the manual approach :/
   
   One rule of thumb I like to use is to add a test whenever I add a new file (like in this case the gluon hybrid implementation) or I fix a bug (considering the CI at that state didn't have tests to properly detect that bug. This way you ensure the bug is now properly caught by our CI in case it occurs again).

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services