marcoabreu edited a comment on issue #13362: Add NHWC layout support to Pooling (cuDNN only) URL: https://github.com/apache/incubator-mxnet/pull/13362#issuecomment-444811649 I would be prefer to leave the booleans in. If there is no harm in taking that approach, I'd rather go down that path and use this operator implementation as example for others rather than waiting for a saint who's going to refactor everything - because that won't happen. Dicks designs often greatly improved the architecture or usability of mxnet, thus I really appreciate these things and definitely would not like to see these improvements blocked because some people can't adapt that fast. This project had some sub-par design decisions that mainly involved choosing preprocessor statements instead of proper class design and abstraction. This PR is a good step towards the first direction and I think everybody should support this path. I'm feeling strong for having this new method in as long as it doesn't have any drawbacks. If there are any, I'm happy to revisit my decision. P.s. I'm not a fan of carrying the data layout outside the data structure, but that's also something we can address in future with a proper class design for ndarray and operators by allowing operators to define their favorite layout and automatic converters from the engine side. If the layout doesn't match (like it's usually the problem for cudnn and mkldnn), it could then be automatically converted into the appropriate format. But this PR is a step towards the right direction, but it can't solve all problems at once. We should appreciate these improvements and I'm sure Dick - or somebody else - will come up with a good design and then provide a reference implementation.
---------------------------------------------------------------- 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
