[GitHub] [incubator-mxnet] ptrendx commented on issue #16553: Fix for wrong reqs set after switching from training to inference
ptrendx commented on issue #16553: Fix for wrong reqs set after switching from training to inference URL: https://github.com/apache/incubator-mxnet/pull/16553#issuecomment-545516103 As expected, test failed in the new test here: http://jenkins.mxnet-ci.amazon-ml.com/job/mxnet-validation/job/centos-gpu/job/PR-15167/50/display/redirect This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] ptrendx commented on issue #16553: Fix for wrong reqs set after switching from training to inference
ptrendx commented on issue #16553: Fix for wrong reqs set after switching from training to inference URL: https://github.com/apache/incubator-mxnet/pull/16553#issuecomment-545206420 Ok, I added the test to #15167 that should fail because of this issue. I will move the string constants to `const static` members of cached op, merge this PR and then merge master to #15167 to show it fixes the problem. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] ptrendx commented on issue #16553: Fix for wrong reqs set after switching from training to inference
ptrendx commented on issue #16553: Fix for wrong reqs set after switching from training to inference URL: https://github.com/apache/incubator-mxnet/pull/16553#issuecomment-544653387 @eric-haibin-lin Yes, it had this bug since the beginning I believe, but I don't think there were any ops that would expose it until now. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] ptrendx commented on issue #16553: Fix for wrong reqs set after switching from training to inference
ptrendx commented on issue #16553: Fix for wrong reqs set after switching from training to inference URL: https://github.com/apache/incubator-mxnet/pull/16553#issuecomment-544336113 For 1: this error was found when working on pointwise fusion, which uses reqs to compile the right code. I don't know if there is any other operator which may not write to an output if there is no backward pass for it (I know `mx.sym.contrib.box_nms` would have that behavior, but the current implementation writes to the output only used for backward every time, without looking at the req value). I can add the test in pointwise fusion PR, show that it fails and then merge with master once this fix is in to show that it does not fail anymore. For 2: I agree, probably as `const static` members of `CachedOp`? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
