apeforest commented on issue #13362: Add NHWC layout support to Pooling (cuDNN only) URL: https://github.com/apache/incubator-mxnet/pull/13362#issuecomment-445008620 @marcoabreu The harm of leaving this boolean return type *in this PR* is that it will cause confusion to developers and also leave unclear messages to MXNet users. E.g. when it returns false, what happens in the Python frontend, any clear message? If this is meant to signal the frontend that this operatator implementation is not supported on particular platform, we need a consistent log message (in fact, we already achieve this via a different way in https://github.com/apache/incubator-mxnet/blob/master/src/operator/nn/mkldnn/mkldnn_pooling.cc#L53). Falling back to the conventional `void` return type seems not affect any functionality of this PR. I would like this PR to mainly focusing on the new feature in Pooling only and we can have a separate PR to change `void` to `boolean` return type and invite more discussions there. I am open to any other suggestions and add @Vikas89 @yuxihu for their comments.
---------------------------------------------------------------- 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: us...@infra.apache.org With regards, Apache Git Services
