Added fp32 / bf16 forward and backward elementwise_div_mkldnn operator #36158

ghost · 2021-09-27T07:59:42Z

PR types

New features

PR changes

OPs

Describe

Adds elementwise_div fp32/bf16 FWD & BWD oneDNN op kernel.

paddle-bot-old · 2021-09-27T07:59:53Z

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

- Refactor BinaryMKLDNNHandler to use a default parameter

jczaja

LGTM

ghost · 2021-09-30T08:07:24Z

@lanxianghit @phlrain Please review and approve.

jakpiase

Apart from some minor comments, really good job! You can also include in title/description, that this kernel is registered for both forward and backward and supports both bf16 and fp32 dtypes

paddle/fluid/operators/elementwise/mkldnn/elementwise_div_mkldnn_op.cc

paddle/fluid/platform/mkldnn_reuse.h

jakpiase · 2021-09-30T21:46:51Z

python/paddle/fluid/tests/unittests/mkldnn/test_elementwise_div_mkldnn_op.py

+        self.out = np.divide(self.x, self.y)
+
+    def test_check_grad_normal(self):
+        pass


Why backward pass is not checked in this scenario?

Backward pass doesn't work correctly with broadcast for the elementwise_mkldnn mul and div operators. I've consulted this with @jczaja and he said that these tests can be disabled for now. I've added a comment to notify the fact that this should be looked into.

paddle/fluid/operators/elementwise/mkldnn/elementwise_div_mkldnn_op.cc

jakpiase · 2021-09-30T22:12:08Z

python/paddle/fluid/tests/unittests/mkldnn/test_elementwise_div_mkldnn_op.py

+        self.out = np.divide(self.x, self.y)
+
+    def test_check_grad_normal(self):
+        self.check_grad(['X', 'Y'], 'Out', max_relative_error=0.05)


5% of relative error is quite a lot, is oneDNN's precision really that bad?

The original elementwise_div tests also used 5%, so that's what I sticked with. In reality, the errors are just over 1%. I've changed the relative error to 2% to make this more clear.

ghost · 2021-10-01T09:51:24Z

@arlesniak Please review

jakpiase

LGTM, really good job

arlesniak

LGTM

arlesniak · 2021-10-07T15:24:41Z

@Xreki Hi, please take a look at this PR, it needs approval (PR-CI-APPROVAL)

arlesniak · 2021-10-08T17:44:31Z

@lidanqing-intel Could you be so kind as to help with approval for this PR, please?

lidanqing-intel

LGTM

paddle-bot-old · 2021-10-15T06:26:59Z

Sorry to inform you that dd1c0ab's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

not accepting max_relative_error

… piotrekobi_elementwise_div_mkldnn

lidanqing-intel · 2021-10-27T03:57:32Z

@piotrekobiIntel Hi, this CheckPRTemplate CI failed do you know why? And Jacek's approval are dismissed because something changed could you please ask his approval and ask him to merge. I have approved. Thanks.

lidanqing-intel

LGTM

PaddlePaddle#36158) * Add WIP version of elementwise_div_mkldnn without working dy grad * Add dy gradient calculation implementation, disable broadcast tests * Readd removed tests from static_mode_white_list * Add bfloat16 gradient tests, remove int8 and uint8 support * - Change the way dy grad is calculated to improve performance - Refactor BinaryMKLDNNHandler to use a default parameter * Change copyright year * Refactor as suggested * Attempt to bypass CI Approval not accepting max_relative_error * Fix formatting issue

Add WIP version of elementwise_div_mkldnn without working dy grad

e529ee0

piotrekobi added 4 commits September 27, 2021 11:39

Add dy gradient calculation implementation, disable broadcast tests

cbc7647

Readd removed tests from static_mode_white_list

2211611

Add bfloat16 gradient tests, remove int8 and uint8 support

330b369

- Change the way dy grad is calculated to improve performance

190a0b9

- Refactor BinaryMKLDNNHandler to use a default parameter

arlesniak added the Intel label Sep 28, 2021

ghost changed the title ~~[WIP] Add elementwise_div_mkldnn~~ Added elementwise_div_mkldnn operator Sep 28, 2021

ghost marked this pull request as ready for review September 28, 2021 14:03

jczaja previously approved these changes Sep 28, 2021

View reviewed changes

Change copyright year

22fc17b

jczaja requested a review from jakpiase September 30, 2021 07:45

jakpiase reviewed Sep 30, 2021

View reviewed changes

jakpiase added the BF16 label Sep 30, 2021

jakpiase reviewed Sep 30, 2021

View reviewed changes

paddle/fluid/operators/elementwise/mkldnn/elementwise_div_mkldnn_op.cc Outdated Show resolved Hide resolved

jakpiase reviewed Sep 30, 2021

View reviewed changes

Refactor as suggested

dd1c0ab

ghost changed the title ~~Added elementwise_div_mkldnn operator~~ Added fp32 / bf16 forward and backward elementwise_div_mkldnn operator Oct 1, 2021

ghost dismissed jczaja’s stale review via dd1c0ab October 1, 2021 08:40

jakpiase previously approved these changes Oct 1, 2021

View reviewed changes

jczaja previously approved these changes Oct 4, 2021

View reviewed changes

arlesniak previously approved these changes Oct 5, 2021

View reviewed changes

lidanqing-intel self-requested a review October 9, 2021 02:33

lidanqing-intel previously approved these changes Oct 9, 2021

View reviewed changes

Attempt to bypass CI Approval

a910f52

not accepting max_relative_error

ghost dismissed stale reviews from lidanqing-intel, arlesniak, jczaja, and jakpiase via a910f52 October 21, 2021 10:12

jczaja previously approved these changes Oct 21, 2021

View reviewed changes

piotrekobi added 2 commits October 25, 2021 10:50

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

c05284c

… piotrekobi_elementwise_div_mkldnn

Fix formatting issue

bf6d2cd

ghost dismissed jczaja’s stale review via bf6d2cd October 26, 2021 08:44

lidanqing-intel approved these changes Oct 27, 2021

View reviewed changes

jczaja merged commit e92e6b0 into PaddlePaddle:develop Oct 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added fp32 / bf16 forward and backward elementwise_div_mkldnn operator #36158

Added fp32 / bf16 forward and backward elementwise_div_mkldnn operator #36158

ghost commented Sep 27, 2021 •

edited by lidanqing-intel

Loading

paddle-bot-old bot commented Sep 27, 2021

jczaja left a comment

ghost commented Sep 30, 2021

jakpiase left a comment •

edited

Loading

jakpiase Sep 30, 2021

ghost Oct 1, 2021 •

edited by ghost

Loading

jakpiase Sep 30, 2021

ghost Oct 1, 2021

ghost commented Oct 1, 2021

jakpiase left a comment

arlesniak left a comment

arlesniak commented Oct 7, 2021

arlesniak commented Oct 8, 2021

lidanqing-intel left a comment

paddle-bot-old bot commented Oct 15, 2021

lidanqing-intel commented Oct 27, 2021 •

edited

Loading

lidanqing-intel left a comment

Added fp32 / bf16 forward and backward elementwise_div_mkldnn operator #36158

Added fp32 / bf16 forward and backward elementwise_div_mkldnn operator #36158

Conversation

ghost commented Sep 27, 2021 • edited by lidanqing-intel Loading

PR types

PR changes

Describe

paddle-bot-old bot commented Sep 27, 2021

jczaja left a comment

Choose a reason for hiding this comment

ghost commented Sep 30, 2021

jakpiase left a comment • edited Loading

Choose a reason for hiding this comment

jakpiase Sep 30, 2021

Choose a reason for hiding this comment

ghost Oct 1, 2021 • edited by ghost Loading

Choose a reason for hiding this comment

jakpiase Sep 30, 2021

Choose a reason for hiding this comment

ghost Oct 1, 2021

Choose a reason for hiding this comment

ghost commented Oct 1, 2021

jakpiase left a comment

Choose a reason for hiding this comment

arlesniak left a comment

Choose a reason for hiding this comment

arlesniak commented Oct 7, 2021

arlesniak commented Oct 8, 2021

lidanqing-intel left a comment

Choose a reason for hiding this comment

paddle-bot-old bot commented Oct 15, 2021

lidanqing-intel commented Oct 27, 2021 • edited Loading

lidanqing-intel left a comment

Choose a reason for hiding this comment

ghost commented Sep 27, 2021 •

edited by lidanqing-intel

Loading

jakpiase left a comment •

edited

Loading

ghost Oct 1, 2021 •

edited by ghost

Loading

lidanqing-intel commented Oct 27, 2021 •

edited

Loading