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:
[email protected]


With regards,
Apache Git Services

Reply via email to