Added elementwise_sub_mkldnn operator #35662

ghost · 2021-09-10T13:40:12Z

PR types

Function optimization

PR changes

OPs

Describe

Added elementwise_sub_mkldnn operator.

CLAassistant · 2021-09-10T13:40:15Z

All committers have signed the CLA.

paddle-bot-old · 2021-09-10T13:40:16Z

✅ This PR's description meets the template requirements!
Please wait for other CI results.

paddle-bot-old · 2021-09-10T13:40:17Z

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

lidanqing-intel · 2021-09-15T12:09:52Z

@piotrekobiIntel Hi, this PR failed on this CI test

021-09-15 03:06:39 failed_test_listsssssss: 
2021-09-15 03:06:39             	333 - test_elementwise_sub_op (Failed)
2021-09-15 03:06:39 ipipe_log_param_Rerunaaaa_TestCases_Total_Time: 304s
2021-09-15 03:06:39 ipipe_log_param_actualaaaaa_TestCases_Total_Time: 3519s
2021-09-15 03:06:39 ========================================
2021-09-15 03:06:39 There are failed tests, which have been executed re-run,but success rate is less than 50%:
2021-09-15 03:06:39 Summary Failed Tests... 
2021-09-15 03:06:39 ========================================
2021-09-15 03:06:39 The following tests FAILED: 
2021-09-15 03:06:39             	333 - test_elementwise_sub_op (Failed)
2021-09-15 03:06:39 + EXCODE=8
2021-09-15 03:06:39 + echo 8
2021-09-15 03:06:39 8
2021-09-15 03:06:39 + echo 'ipipe_log_param_EXCODE: 8'
2021-09-15 03:06:39 ipipe_log_param_EXCODE: 8
2021-09-15 03:06:39 + '[' 8 -ne 0 ']'
2021-09-15 03:06:39 + '[' 8 -ne 9 ']'
2021-09-15 03:06:39 + exit 8
2021-09-15 03:06:39 {build code state=8}

Do you know how to run unit test locally? Just do ctest -R test_elementwise_sub_op -V, For any help, please contact Jacek, Joanna or me. And we would like to merge this PR before 17th September which is new branch day, so just contact me if you need any help. Thanks

jakpiase · 2021-09-20T09:20:11Z

python/paddle/fluid/tests/unittests/test_elementwise_sub_op.py

+    def init_input_output(self):
+        self.x = np.random.rand(2, 3, 4).astype(self.dtype)
+        self.y = np.random.rand(1, 1).astype(self.dtype)
+        self.out = self.x - self.y




Suggested change

@OpTestTool.skip_if(True, "Grad not yet implemented")

You have disabled the upper test, now CI fails on that one having no FP16 kernel

And from what I can see in CI, double grad is causing problems, not just single grad

Most of the elementwise_sub tests will be removed anyway, because they were copied from elementwise_add and it turned out that most are just not supported. Right now I'm working on making sure that grad tests work with the mkldnn implementation, because there were disabled until now.

ghost · 2021-09-20T09:27:17Z

@piotrekobiIntel Hi, this PR failed on this CI test

021-09-15 03:06:39 failed_test_listsssssss: 
2021-09-15 03:06:39             	333 - test_elementwise_sub_op (Failed)
2021-09-15 03:06:39 ipipe_log_param_Rerunaaaa_TestCases_Total_Time: 304s
2021-09-15 03:06:39 ipipe_log_param_actualaaaaa_TestCases_Total_Time: 3519s
2021-09-15 03:06:39 ========================================
2021-09-15 03:06:39 There are failed tests, which have been executed re-run,but success rate is less than 50%:
2021-09-15 03:06:39 Summary Failed Tests... 
2021-09-15 03:06:39 ========================================
2021-09-15 03:06:39 The following tests FAILED: 
2021-09-15 03:06:39             	333 - test_elementwise_sub_op (Failed)
2021-09-15 03:06:39 + EXCODE=8
2021-09-15 03:06:39 + echo 8
2021-09-15 03:06:39 8
2021-09-15 03:06:39 + echo 'ipipe_log_param_EXCODE: 8'
2021-09-15 03:06:39 ipipe_log_param_EXCODE: 8
2021-09-15 03:06:39 + '[' 8 -ne 0 ']'
2021-09-15 03:06:39 + '[' 8 -ne 9 ']'
2021-09-15 03:06:39 + exit 8
2021-09-15 03:06:39 {build code state=8}

Do you know how to run unit test locally? Just do ctest -R test_elementwise_sub_op -V, For any help, please contact Jacek, Joanna or me. And we would like to merge this PR before 17th September which is new branch day, so just contact me if you need any help. Thanks

I ran the unit tests locally before pushing and they passed, they only failed in the CI for some reason. Most of the elementwise_sub tests will be removed though, after I'm done implementing mkldnn grad support.

jakpiase · 2021-09-20T09:30:08Z

@piotrekobiIntel some machines are testing CUDA implementation and maybe that is causing problem? For example when CUDA is running, bfloat16 tests must be disabled, maybe float16 must be disabled as well?

jakpiase · 2021-09-20T09:33:16Z

@piotrekobiIntel

You have to disable float16 tests if PLACE== CUDA, as you can see in this image" CUDAPlace" was running and caused an error.
You can disable CUDA with OpTestTool.skip_if_not_cpu_bf16()

This reverts commit fc3b122.

…ddlePaddle#35862)

arlesniak

Thank you for contribution

paddle/fluid/platform/mkldnn_reuse.h

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

…atisfy Approval CI

* Add elementwise_sub_mkldnn_op without grad * Add test to static_mode_white_list * Refactor code, change license years * Remove invalid grad implementation * Fix element_wise_sub_op test * Fix CI Approval error * Remove unnecessary EltwiseSubMKLDNNGradKernel class * Fix CI Approval 2 * Fix CI Approval 3 * Fix CI Approval Attempt #4 * Fix CI Approve Attempt #5 * Fix CI Approval Attempt #6 * Fix CI Approval Attemt #7 * Change test names containing add to sub * Fix old tests testing add instead of sub * Copy grad implementation from elementwise_add_mkldnn * CI test fix attempt * Revert "CI test fix attempt" This reverts commit c647cacf41e6a87c715385a185de5cbf65fc8900. * Fix CI attempt 2 * Fix elementwise_sub tests, temporary mkldnn broadcast test disable * Add working implementation of elementwise_sub grad * Fix build errors caused by pull * Fix format error * Fix format error 2 * Disable elementwise_sub_mkldnn test on GPU * Apply fix for paddle.fluid import * Revert changes of test_elementwise_sub and Fix mkldnn test * Revert "Apply fix for paddle.fluid import" This reverts commit fc3b122. * fix bug of module 'paddle' has no attribute 'fluid' for python3.6 (PaddlePaddle#35862) * Add changes suggested by reviewers * Change @unittest.skipIf... to @OpTestTool.skip_if_not_cpu_bf16() to satisfy Approval CI * Remove check_dygraph=False to satisify CI Approval Co-authored-by: zhangbo9674 <82555433+zhangbo9674@users.noreply.github.com>

arlesniak added the Intel label Sep 10, 2021

jakpiase reviewed Sep 20, 2021

View reviewed changes

piotrekobi added 4 commits September 20, 2021 12:55

Add elementwise_sub_mkldnn_op without grad

b5374a1

Add test to static_mode_white_list

97b2293

Refactor code, change license years

8f56b90

Remove invalid grad implementation

2b1be1c

piotrekobi added 2 commits September 20, 2021 12:57

Fix elementwise_sub tests, temporary mkldnn broadcast test disable

fc02000

Add working implementation of elementwise_sub grad

b4d7c9e

ghost force-pushed the elementwise_sub_mkldnn_implement branch from 287d5e8 to b4d7c9e Compare September 20, 2021 12:17

piotrekobi added 7 commits September 21, 2021 09:46

Fix build errors caused by pull

a6822c6

Fix format error

57fe561

Fix format error 2

557ff38

Disable elementwise_sub_mkldnn test on GPU

314f214

Apply fix for paddle.fluid import

fc3b122

Revert changes of test_elementwise_sub and Fix mkldnn test

56852cd

Revert "Apply fix for paddle.fluid import"

0dcc8e2

This reverts commit fc3b122.

ghost marked this pull request as ready for review September 22, 2021 10:14

arlesniak requested review from jczaja, jakpiase, arlesniak and arogowie-intel September 22, 2021 10:15

fix bug of module 'paddle' has no attribute 'fluid' for python3.6 (Pa…

9c98cc8

…ddlePaddle#35862)

arlesniak added Intel and removed Intel labels Sep 22, 2021

ghost changed the title ~~[WIP] add elementwise_sub_mkldnn operator~~ Added elementwise_sub_mkldnn operator Sep 22, 2021

arlesniak reviewed Sep 22, 2021

View reviewed changes

paddle/fluid/platform/mkldnn_reuse.h Outdated Show resolved Hide resolved

arogowie-intel previously approved these changes Sep 22, 2021

View reviewed changes

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

jakpiase reviewed Sep 22, 2021

View reviewed changes

piotrekobi added 3 commits September 23, 2021 09:31

Add changes suggested by reviewers

ea395f5

Change @unittest.skipIf... to @OpTestTool.skip_if_not_cpu_bf16() to s…

f3010a0

…atisfy Approval CI

Remove check_dygraph=False to satisify CI Approval

08a5c69

ghost dismissed arogowie-intel’s stale review via ea395f5 September 23, 2021 10:47

jczaja approved these changes Sep 24, 2021

View reviewed changes

jczaja merged commit 787273e into PaddlePaddle:develop Sep 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added elementwise_sub_mkldnn operator #35662

Added elementwise_sub_mkldnn operator #35662

ghost commented Sep 10, 2021 •

edited by ghost

Loading

CLAassistant commented Sep 10, 2021 •

edited

Loading

paddle-bot-old bot commented Sep 10, 2021 •

edited

Loading

paddle-bot-old bot commented Sep 10, 2021

lidanqing-intel commented Sep 15, 2021 •

edited

Loading

jakpiase Sep 20, 2021

jakpiase Sep 20, 2021

ghost Sep 20, 2021

ghost commented Sep 20, 2021

jakpiase commented Sep 20, 2021

jakpiase commented Sep 20, 2021 •

edited

Loading

arlesniak left a comment

Added elementwise_sub_mkldnn operator #35662

Added elementwise_sub_mkldnn operator #35662

Conversation

ghost commented Sep 10, 2021 • edited by ghost Loading

PR types

PR changes

Describe

CLAassistant commented Sep 10, 2021 • edited Loading

paddle-bot-old bot commented Sep 10, 2021 • edited Loading

paddle-bot-old bot commented Sep 10, 2021

lidanqing-intel commented Sep 15, 2021 • edited Loading

jakpiase Sep 20, 2021

Choose a reason for hiding this comment

jakpiase Sep 20, 2021

Choose a reason for hiding this comment

ghost Sep 20, 2021

Choose a reason for hiding this comment

ghost commented Sep 20, 2021

jakpiase commented Sep 20, 2021

jakpiase commented Sep 20, 2021 • edited Loading

arlesniak left a comment

Choose a reason for hiding this comment

ghost commented Sep 10, 2021 •

edited by ghost

Loading

CLAassistant commented Sep 10, 2021 •

edited

Loading

paddle-bot-old bot commented Sep 10, 2021 •

edited

Loading

lidanqing-intel commented Sep 15, 2021 •

edited

Loading

jakpiase commented Sep 20, 2021 •

edited

Loading