Good catch. In general, I agree that tests are not supposed to removed, although CI is not running any of these cpp tests just yet. Usually unit tests in python for individual operators should be sufficient to test the correctness of operators (although I don't know how/if python tests can run on edge devices like iOS/Android) and it's not feasible to duplicate all operator python tests with their cpp counterparts. For functionalities that are not exposed to python or memory checks, I do agree that cpp tests are necessary.
I am curious what are exactly tested in those cpp tests? I know that BatchNorm ops are tested in python already, but I don't have a full picture of all cpp tests and whether we want to re-implement them. On the other hand, be aware that the Intel team have a few MKL-DNN related projects depending on this PR. Reverting the PR will be a blocker for them. Best, Haibin On Wed, Jan 31, 2018 at 3:30 PM, Marco de Abreu < marco.g.ab...@googlemail.com> wrote: > Here's a link: > https://github.com/apache/incubator-mxnet/pull/8302#discussion_r165204667 > > -Marco > > 2018-01-31 15:23 GMT-08:00 Marco de Abreu <marco.g.ab...@googlemail.com>: > > > Hi Chris, > > > > considering the size of that PR, could you provide a direct link to the > > changes you're addressing? > > > > -Marco > > > > 2018-01-31 13:39 GMT-08:00 Chris Olivier <cjolivie...@gmail.com>: > > > >> This PR was just merged that removed some 30 or so C++ unit tests for > >> batch > >> norm operator. > >> > >> https://github.com/apache/incubator-mxnet/pull/8302 > >> > >> Is this ok? > >> > > > > >
