Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Sum kernel for CPU supporting BF16 and SelectedRows #32631

Merged
merged 20 commits into from
May 6, 2021
Merged

Sum kernel for CPU supporting BF16 and SelectedRows #32631

merged 20 commits into from
May 6, 2021

Conversation

arogowie-intel
Copy link
Contributor

PR types

New features

PR changes

OPs

Describe

Introduce CPU native kernel supporting SUM operator with BF16 data type and SelectedRows input tensors.

@paddle-bot-old
Copy link

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@arogowie-intel arogowie-intel self-assigned this Apr 27, 2021
jczaja
jczaja reviewed Apr 28, 2021
View reviewed changes
paddle/fluid/platform/mkldnn_reuse.h Outdated Show resolved Hide resolved
paddle/fluid/operators/math/blas_impl.h Outdated Show resolved Hide resolved
jczaja
jczaja previously approved these changes Apr 29, 2021
View reviewed changes
Copy link
Contributor

@jczaja jczaja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@arogowie-intel
Remove not used if branch.
e77c8ac 
* reported by coverage CI.
arlesniak
arlesniak previously approved these changes May 5, 2021
View reviewed changes
Copy link
Contributor

@arlesniak arlesniak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@arogowie-intel
Copy link
Contributor Author

arogowie-intel commented May 5, 2021 •
edited
Loading

@luotao1 The CI failures are as following:

  1. PR-CI-APPROVAL is because I'm skipping UT on platforms not supporting BF16 data type.
  2. PR-CI-Coverage is IMHO a false positive, since I can set a breakpoint and stop in gdb in lines which it doesn't see. Attached file with backtrace: sum_op_py_bt.log command: gdb --args python python/paddle/fluid/tests/unittests/test_sum_op.py TestSelectedRowsSumOpInt
  3. PR-CI-OP-benchmark is running GPU benchmarks and this PR's changes are not related to GPU.

lidanqing-intel
lidanqing-intel reviewed May 5, 2021
View reviewed changes
Copy link
Contributor

@lidanqing-intel lidanqing-intel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some doubts. Thanks

paddle/fluid/platform/mkldnn_reuse.h Outdated Show resolved Hide resolved
paddle/fluid/platform/mkldnn_reuse.h Outdated Show resolved Hide resolved
@lidanqing-intel
Copy link
Contributor

LGTM.

@arogowie-intel
Copy link
Contributor Author

@luotao1 Could you start your review please? I've explained in earlier comment why some CI runs failed.

@luotao1 luotao1 merged commit 9599c3b into PaddlePaddle:develop May 6, 2021
@luotao1
Copy link
Contributor

luotao1 commented May 6, 2021

Please cherry-pick into release/2.1

@arogowie-intel arogowie-intel deleted the aosewski/sum_selected_rows branch May 6, 2021 09:09
arogowie-intel added a commit to arogowie-intel/Paddle that referenced this pull request May 6, 2021
@arogowie-intel
Sum kernel for CPU supporting BF16 and SelectedRows (PaddlePaddle#32631)
d428468
@arogowie-intel arogowie-intel mentioned this pull request May 6, 2021
Merged
lanxianghit pushed a commit that referenced this pull request May 6, 2021
@arogowie-intel
[cherry-pick] Sum kernel for CPU supporting BF16 and SelectedRows (#3…
f3436af 
…2631) (#32755)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luotao1 luotao1 luotao1 approved these changes

@jczaja jczaja jczaja left review comments

@arlesniak arlesniak arlesniak left review comments

@wozna wozna Awaiting requested review from wozna

@lidanqing-intel lidanqing-intel Awaiting requested review from lidanqing-intel

Assignees

@luotao1 luotao1

@arogowie-intel arogowie-intel

Labels
BF16 Intel
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@arogowie-intel @lidanqing-intel @luotao1 @jczaja @arlesniak