The output format of mkldnn conv is wrong when data_format is NHWC

[baoachun]

The problem can be reproduced according to this pr：#38107

Runing the following command to reproduce.

git clone the code and cmake with -DWITH_TESTING=ON
then running the following command

ctest -R test_mkldnn_conv_gelu_fuse_pass -V

[paddle-bot-old]

您好，我们已经收到了您的问题，会安排技术人员尽快解答您的问题，请耐心等待。请您再次检查是否提供了清晰的问题描述、复现代码、环境&版本、报错信息等。同时，您也可以通过查看官网API文档、常见问题、历史Issue、AI社区来寻求解答。祝您生活愉快～

Hi! We've received your issue and please be patient to get responded. We will arrange technicians to answer your questions as soon as possible. Please make sure that you have posted enough message to demo your request. You may also check out the API，FAQ，Github Issue and AI community to get the answer.Have a nice day!

[lidanqing-intel]

@jczaja Please comment on this issue. Otherwise that disabling PR will be merged.

[jczaja]

@baoachun I implemented NHWC support for oneDNN kernels in PaddlePaddle so I can offer some insight.
If PaddlePaddle input is of layout NCHW then shape is [N,C,H,W]. If PaddlePaddle input is of layout NHWC then shape is [N,H,W,C]. For oneDNN kernels input regardless if it is NCHW or NHWC then shape is always [N,C,H,W]. So internally for NHWC models when there is a CPU op followed by oneDNN op then there is shape rotation from [N,H,W,C] to [N,C,H,W]. And when there is oneDNN op followed by CPU op then there is shape roatation from [N,C, H, W] to [N,H,W,C]. The thing is that this assumes that after oneDNN op there is some CPU op. When We implemented NHWC support then there was fetch op always the last one so shape rotation happened there. But I noticed that later on PaddlePaddle by default is skipping fetch op and user get access to data that has still oneDNN shape. So proper fix would be to add this shape rotation in a mechanics where you get access to oneDNN tensor. Here is line calling shape rotation:

Paddle/paddle/fluid/framework/data_layout_transform.cc

Lines 189 to 191 in 21b7ed3

	// For exepected NHWC data format we need to reshape the Output tensor
	// As MKL-DNN description was in NCHW and paddle is expecting NHWC
	platform::MatchShapeToLayout(out, in_layout, out_layout);

Some more information (design doc on NHWC in oneDNN kernels):
https://github.com/PaddlePaddle/docs/blob/develop/docs/design/mkldnn/nhwc/nhwc.md

[baoachun]

Hi @jczaja , at present, a user encounters the inference error problem, which is caused because conv2d does not support NHWC format.

am_veo.tar.gz

[jczaja]

@baoachun Thanks for more information. I'm planning to work on this issue right after I finish current issue so I will start work on NHWC next week.

[jczaja]

@baoachun Just to let You know that I started to investigate this issue. Reproduction was achieved. Problem seems to be that oneDNN elementwise_add (X,Y) kernel with Y being broadcasted , does not work properly for NHWC situation. I will write some more when I get more details.

[jczaja]

@baoachun Situation is as follows. There are two problems encountered:

1. broadcasting in oneDNN's elementwise kernels for NHWC is not working properly. Candidate fix is here : The output format of mkldnn conv is wrong when data_format is NHWC #38126 .
2. batch norm of NHWC in oneDNN's kernel does not work properly for 3D data. Status: under investigaton

[jczaja]

@baoachun I just rebased #39097 and it does make this issue to go away. So please test it if this PR #39097 works for you.

[baoachun]

Hi @jczaja , most of the UTs have passed, except for test_mkldnn_conv_elementwise_add_fuse_pass, the error message is as follows. I noticed that the default input in the conv operator is NCHW format. Is this judgment reasonable?

Paddle/paddle/fluid/operators/conv_op.cc

Line 60 in 0d46a10

const bool channel_last = (ctx->IsRunMKLDNNKernel() == false) &&

[jczaja]

@baoachun Ok, I understand that in #38107 there is UT added with ignored NHWC and I should enable and fix that one. I will start working on this.

[jczaja]

@baoachun Not sure if I'm solving proper issue, but I enabled NHWC part of UT in test checking fusing pass of conv and gelu. PR is here : #39591 . Check if this is what you have expected to fix?

[baoachun]

Hi @jczaja , most of the UTs have passed, except for test_mkldnn_conv_elementwise_add_fuse_pass, the error message is as follows. I noticed that the default input in the conv operator is NCHW format. Is this judgment reasonable?

Paddle/paddle/fluid/operators/conv_op.cc

Line 60 in 0d46a10

const bool channel_last = (ctx->IsRunMKLDNNKernel() == false) &&

Hi @jczaja, I have removed most of the skip settings for NHWC, pr is here: #39551, but the test_mkldnn_conv_elementwise_add_fuse_pass UT failed, ans I got the above error message, I'm not sure this judgment is reasonable at this time.

[jczaja]

@baoachun Ok, so to reproduce this problem I should use enable NHWC in test_mkldnn_conv_elementwise_add_fuse_pass ?

[baoachun]

Hi @jczaja, you can use this pr #39654 to reproduce.

[jczaja]

The issues reported here are fixed by #40049 . Please retest and let us now if all works for you

[lidanqing-intel]

@baoachun said PaddleHub reported failures after the PR merged. Hence please keep this issue open. More info will follow

[lidanqing-intel]

To summarize:

1. The model error has been fixed by oneDNN NHWC fixes #40049, However, a newer Baidu commit broke the model again Advice on model that stopped to work #40540
2. We expect the commit author could fix the issue, if you need any support or explanation of oneDNN NHWC fixes #40049, @jczaja will answer in time.

[yaomichael]

notes from 5/20 meeting:
The regression caused by PaddleHub team should have been fixed already. @jiangjiajun will check internally and close this ticket.