oneDNN NHWC fixes #40049
Merged
oneDNN NHWC fixes #40049
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for your contribution! |
jczaja
force-pushed
the
prv-nhwc-yet-another-implementation
branch
from
1763e97 to
70869b8
Compare
jczaja
force-pushed
the
prv-nhwc-yet-another-implementation
branch
from
1db6f35 to
e576182
Compare
jczaja
force-pushed
the
prv-nhwc-yet-another-implementation
branch
from
e576182 to
72fd182
Compare
- fix - compilation fixes - fix - fixe - fix - fix - compilation fix - comment fix - lint update mkldnn conv_elementwise_add_fuse_pass ut - NHWC changes to prelu - alhpa dims - UT fix - fix to UT - lint - Some fixes - added to BWD of prelu NHWC support - reverted removal of resetting cu_layout in clearing of caching
|
@baoachun Hi, this PR passed all CIs please review. |
tsocha
suggested changes
Mar 10, 2022
Comment on lines
566
to
567
| auto check_attrib = [&](std::unique_ptr<framework::OperatorBase>& op, | ||
| const std::string& attrib_name) -> bool { |
There was a problem hiding this comment.
Suggested change
| auto check_attrib = [&](std::unique_ptr<framework::OperatorBase>& op, | |
| const std::string& attrib_name) -> bool { | |
| auto check_attrib = [](std::unique_ptr<framework::OperatorBase>& op, | |
| const std::string& attrib_name) -> bool { |
python/paddle/fluid/tests/unittests/ir/inference/test_mkldnn_conv_elementwise_add_fuse_pass.py
Show resolved
Hide resolved
paddle/fluid/operators/prelu_op.cc
Outdated
Comment on lines
234
to
250
| framework::OpKernelType GetKernelTypeForVar( | ||
| const std::string &var_name, const Tensor &tensor, | ||
| const framework::OpKernelType &expected_kernel_type) const { | ||
| #ifdef PADDLE_WITH_MKLDNN | ||
| // All inputs (including alpha) need shape rotating | ||
| if ((expected_kernel_type.data_layout_ == framework::DataLayout::kMKLDNN) && | ||
| (tensor.layout() != framework::DataLayout::kMKLDNN) && | ||
| paddle::platform::MKLDNNDeviceContext::tls() | ||
| .get_cur_paddle_data_layout() == framework::DataLayout::kNHWC) { | ||
| return framework::OpKernelType(expected_kernel_type.data_type_, | ||
| tensor.place(), | ||
| framework::DataLayout::kNHWC); | ||
| } | ||
| #endif | ||
| return framework::OpKernelType(expected_kernel_type.data_type_, | ||
| tensor.place(), tensor.layout()); | ||
| } |
There was a problem hiding this comment.
Don't repeat yourself, please.
tsocha
previously approved these changes
Mar 10, 2022
Silv3S
reviewed
Mar 10, 2022
| def is_program_valid(self, program_config: ProgramConfig) -> bool: | ||
| attrs = [ | ||
| program_config.ops[i].attrs | ||
| for i in range(len(program_config.ops)) | ||
| ] | ||
| # If the problem has been fixed, the judgment |
There was a problem hiding this comment.
Similar condition is in test_mkldnn_conv_mish_fuse_pass.py and test_mkldnn_depthwise_conv.py. Should we enable testing NHWC there too?
There was a problem hiding this comment.
@Silv3S This UT was added in #39654 So I just copied it here as suggessted by @lidanqing-intel . I do not intend to extend it in this PR.
| quant=False, passes=["conv_elementwise_add_mkldnn_fuse_pass"]) | ||
|
|
||
|
|
||
| ''' |
There was a problem hiding this comment.
Why is whole original unit test commented?
There was a problem hiding this comment.
I just copied this UT from #39654 as requested so not sure of authors intention
tsocha
approved these changes
Mar 10, 2022
|
LGTM |
|
@baoachun Please review. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR types
Bug fixes
PR changes
Others
Describe
oneDNN NHWC implementation need to know upfront if model is NHWC or NCHW so mechanics was added to check that.
Also relevant UT was added from #39654