[oneDNN] Refactoring of softmax grad onednn kernel to match common API #32851
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
requested review from
arogowie-intel,
jakpiase,
arlesniak,
wozna and
lidanqing-intel
jakpiase
suggested changes
May 13, 2021
| auto* dout = ctx.template Input<Tensor>(framework::GradVarName("Out")); | ||
| auto* dx = | ||
| auto* out_grad = ctx.template Input<Tensor>(framework::GradVarName("Out")); | ||
| auto* in_x_grad = | ||
| ctx.template Output<framework::Tensor>(framework::GradVarName("X")); |
There was a problem hiding this comment.
Suggested change
| ctx.template Output<framework::Tensor>(framework::GradVarName("X")); | |
| ctx.template Output<Tensor>(framework::GradVarName("X")); |
Please, stay consistent with the usage of namespaces
|
|
||
| auto dims = out_grad->dims(); // input and output share the same shape | ||
| const int axis = CanonicalAxis(ctx.Attr<int>("axis"), dims.size()); | ||
| auto softmax_tz = paddle::framework::vectorize<int64_t>(dims); |
There was a problem hiding this comment.
Suggested change
| auto softmax_tz = paddle::framework::vectorize<int64_t>(dims); | |
| auto softmax_tz = framework::vectorize<int64_t>(dims); |
| SoftmaxMKLDNNHandler(const std::vector<int64_t>& dims, | ||
| const MKLDNNMemoryFormat fmt, | ||
| const MKLDNNMemoryFormat diff_fmt, const int& axis, | ||
| SoftmaxMKLDNNHandler(const paddle::framework::ExecutionContext& ctx, |
There was a problem hiding this comment.
Suggested change
| SoftmaxMKLDNNHandler(const paddle::framework::ExecutionContext& ctx, | |
| SoftmaxMKLDNNHandler(const framework::ExecutionContext& ctx, |
| SoftmaxMKLDNNHandler(const std::vector<int64_t>& dims, | ||
| const MKLDNNMemoryFormat fmt, | ||
| const MKLDNNMemoryFormat diff_fmt, const int& axis, | ||
| SoftmaxMKLDNNHandler(const paddle::framework::ExecutionContext& ctx, | ||
| const platform::MKLDNNDeviceContext& dev_ctx, |
There was a problem hiding this comment.
Suggested change
| const platform::MKLDNNDeviceContext& dev_ctx, | |
| const MKLDNNDeviceContext& dev_ctx, |
Since you have using "paddle::platform::MKLDNNDeviceContext;" in line 33, you don't need to declare this namespace here
| const int axis = CanonicalAxis(ctx.Attr<int>("axis"), dims.size()); | ||
| auto softmax_tz = paddle::framework::vectorize<int64_t>(dims); | ||
|
|
||
| auto data_softmax_md = platform::MKLDNNMemDesc( |
There was a problem hiding this comment.
Suggested change
| auto data_softmax_md = platform::MKLDNNMemDesc( | |
| auto data_softmax_md = MKLDNNMemDesc( |
|
|
||
| auto data_softmax_md = platform::MKLDNNMemDesc( | ||
| softmax_tz, platform::MKLDNNGetDataType<T>(), out->format()); | ||
| auto diff_softmax_md = platform::MKLDNNMemDesc( |
There was a problem hiding this comment.
Suggested change
| auto diff_softmax_md = platform::MKLDNNMemDesc( | |
| auto diff_softmax_md = MKLDNNMemDesc( |
lidanqing-intel
approved these changes
May 14, 2021
| auto softmax_tz = framework::vectorize<int64_t>(dims); | ||
|
|
||
| auto data_softmax_md = MKLDNNMemDesc( | ||
| softmax_tz, platform::MKLDNNGetDataType<T>(), out->format()); |
There was a problem hiding this comment.
I have one doubt:
out->format and out_grad->format will be "NHWC" or "NCHW"? which is Paddle format. In this case if next op is also mkldnn op, is reorder needed ?
|
@luotao1 Could you please start your 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
Function optimization
PR changes
OPs
Describe
This PR modifies softmax grad oneDNN kernel so its implementation of other oneDNN grad kernels. This is needed for bigger changes that will come in next PRs