[PHI] migrating Squeeze2 and reshape2 #46331

sfraczek · 2022-09-20T15:53:07Z

PR types

Function optimization

PR changes

OPs

Describe

Migration of reshape2 and squeeze2 from fluid to phi.

also add xshape version to kernel

paddle-bot · 2022-09-20T15:53:11Z

你的PR提交成功，感谢你对开源项目的贡献!
请关注后续CI自动化测试结果，详情请参考Paddle-CI手册。
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

by explicitly specifying template args to call to ReshapeKernel in ReshapeWithXShape

… macro registration

paulinagacek

LGTM

Silv3S · 2022-09-23T13:07:04Z

paddle/fluid/operators/mkldnn/reshape_mkldnn_op.cc

@@ -104,21 +104,16 @@ class ReshapeMKLDNNKernel : public framework::OpKernel<T> {
  }

  void InferInOutShape(const framework::ExecutionContext& ctx,
+


remove empty line

Silv3S · 2022-09-23T13:15:14Z

paddle/phi/kernels/onednn/reshape_kernel.cc

+              capacity));
+    }
+  }
+  return make_ddim(output_shape);


This method is already implemented in paddle/phi/infermeta/unary.cc. Maybe it's better to import header and call that implementation instead of copying it here?

This method is not exported in header (unary.h) so it's invisible.

YuanRisheng

The CI-coverage is not successful. Please check if the ReshapeKernel and SqueezeKernel are called. Check these unittests:test_reshape_mkldnn_op and test_squeeze2_mkldnn_op with Paddle that is not compiled with cuda. If they are called, I will mark this ci as successful. Otherwise, I will help you to find the reason why they are not called.

sfraczek · 2022-09-26T16:00:13Z

The CI-coverage is not successful. Please check if the ReshapeKernel and SqueezeKernel are called. Check these unittests:test_reshape_mkldnn_op and test_squeeze2_mkldnn_op with Paddle that is not compiled with cuda. If they are called, I will mark this ci as successful. Otherwise, I will help you to find the reason why they are not called.

When I run the test test_reshape_mkldnn_op I have 4 errors on my branch and on develop the same. The errors regard bfloat on plain CPU library. They say something about missing shape variable.
I see only the default cpu reshape kernel executed. Old and new mkldnn kernel is not executed. Please help me find out why. I haven't checked squeeze yet.

sfraczek · 2022-09-27T10:08:28Z

@YuanRisheng I would also like to register this kernel only for float and bfloat but I didn't know how, this macro seems to ignore dtype.

YuanRisheng · 2022-09-27T12:34:16Z

@YuanRisheng I would also like to register this kernel only for float and bfloat but I didn't know how, this macro seems to ignore dtype.

Well, I find out the truth of the bug:#35781. The mkldnn kernel of Reshape2 and Squeeze2 have bugs and have been disabled last year. So, there are 2 options you can choose:
1, Delete the code of Reshape2 and Squeeze2 and they will not migrated to PHI.
2, Fix these two kernels' bugs and migrate to PHI

I suggest the first option, because these two kernels are not used at all now and it is simple. @sfraczek

sfraczek · 2022-09-29T12:55:50Z

@YuanRisheng I would also like to register this kernel only for float and bfloat but I didn't know how, this macro seems to ignore dtype.

Well, I find out the truth of the bug:#35781. The mkldnn kernel of Reshape2 and Squeeze2 have bugs and have been disabled last year. So, there are 2 options you can choose: 1, Delete the code of Reshape2 and Squeeze2 and they will not migrated to PHI. 2, Fix these two kernels' bugs and migrate to PHI

I suggest the first option, because these two kernels are not used at all now and it is simple. @sfraczek

Thank you. We will try to fix it first and delay the migration.

sfraczek · 2022-11-28T13:44:26Z

waiting for #48359

$sfraczek$

sfraczek added 4 commits September 20, 2022 12:05

$@sfraczek$

add phi reshape kernel code

7e2643b

$@sfraczek$

fix building error

fae11e7

$@sfraczek$

remove old kernel registration and add phi registration

d274ca3

$@sfraczek$

registration takes no <>

74820ad

also add xshape version to kernel

paddle-bot bot added contributor External developers status: proposed labels Sep 20, 2022

Silv3S added the Intel label Sep 20, 2022

paddle-bot bot removed the status: proposed label Sep 20, 2022

Silv3S mentioned this pull request Sep 21, 2022

Migrate oneDNN operators to under PHI #45308

Closed

sfraczek added 2 commits September 21, 2022 14:59

$@sfraczek$

add template typename T to kernel signature

c4958d1

$@sfraczek$

fix another error

91a5da1

by explicitly specifying template args to call to ReshapeKernel in ReshapeWithXShape

$@sfraczek$ sfraczek requested a review from jakpiase September 21, 2022 13:09

sfraczek added 12 commits September 21, 2022 15:43

$@sfraczek$

formatting

6e74454

$@sfraczek$

remove unncessary lines

8d033c8

$@sfraczek$

remove reshape only code from reshape_mkldnn_op.cc

9585909

$@sfraczek$

migrate squeeze2

3382b31

$@sfraczek$

revert empty line removal

d6d99c7

$@sfraczek$

review for reshape

8db5cdc

$@sfraczek$

fixes to squeeze

44840ad

$@sfraczek$

revert removing reshape registration

6262303

$@sfraczek$

revert removing reshape function

cbd20b0

$@sfraczek$

fix logged message

49059a0

$@sfraczek$

Merge remote-tracking branch 'origin/develop' into squeeze2-reshape2

64ad7be

$@sfraczek$

revert removing squeeze1 code

dadc6c0

$@sfraczek$

This comment was marked as resolved.

Sign in to view

$@sfraczek$

remove logging

fb230d8

$@sfraczek$ sfraczek marked this pull request as ready for review September 23, 2022 08:49

$@sfraczek$ sfraczek requested review from Silv3S and piotrekobi September 23, 2022 08:50

sfraczek added 2 commits September 23, 2022 10:06

$@sfraczek$

remove phi:: from method calls and remove T from templates and change…

d739d24

… macro registration

$@sfraczek$

remove phi prefix in squeeze

e4b911c

paulinagacek previously approved these changes Sep 23, 2022

View reviewed changes

Silv3S reviewed Sep 23, 2022

View reviewed changes

$@sfraczek$ sfraczek requested a review from chenwhql September 23, 2022 13:39

YuanRisheng reviewed Sep 26, 2022

View reviewed changes

$@sfraczek$

remove empty line

561d5a8

$@sfraczek$ sfraczek dismissed paulinagacek’s stale review via 561d5a8 September 29, 2022 12:54

paddle-bot-old bot removed the contributor External developers label Oct 17, 2022

onecatcn assigned chenwhql and iclementine Nov 14, 2022

sfraczek added 5 commits November 30, 2022 11:28

$@sfraczek$

Merge branch 'develop' into squeeze2-reshape2

fba7760

$@sfraczek$

fixes after merge

54295c8

$@sfraczek$

fix

79ec128

$@sfraczek$

fix

3075b80

$@sfraczek$

Merge branch 'PaddlePaddle:develop' into squeeze2-reshape2

3b7bc7a

Silv3S closed this Dec 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PHI] migrating Squeeze2 and reshape2 #46331

[PHI] migrating Squeeze2 and reshape2 #46331

$@sfraczek$ sfraczek commented Sep 20, 2022 •

edited

Loading

paddle-bot bot commented Sep 20, 2022

This comment was marked as resolved.

paulinagacek left a comment

Silv3S Sep 23, 2022

Silv3S Sep 23, 2022

$@sfraczek$ sfraczek Sep 26, 2022

YuanRisheng left a comment

sfraczek commented Sep 26, 2022 •

edited

Loading

sfraczek commented Sep 27, 2022

YuanRisheng commented Sep 27, 2022 •

edited

Loading

sfraczek commented Sep 29, 2022

sfraczek commented Nov 28, 2022

		@@ -104,21 +104,16 @@ class ReshapeMKLDNNKernel : public framework::OpKernel<T> {
		}

		void InferInOutShape(const framework::ExecutionContext& ctx,

[PHI] migrating Squeeze2 and reshape2 #46331

[PHI] migrating Squeeze2 and reshape2 #46331

Conversation

sfraczek commented Sep 20, 2022 • edited Loading

PR types

PR changes

Describe

paddle-bot bot commented Sep 20, 2022

This comment was marked as resolved.

paulinagacek left a comment

Choose a reason for hiding this comment

Silv3S Sep 23, 2022

Choose a reason for hiding this comment

Silv3S Sep 23, 2022

Choose a reason for hiding this comment

sfraczek Sep 26, 2022

Choose a reason for hiding this comment

YuanRisheng left a comment

Choose a reason for hiding this comment

sfraczek commented Sep 26, 2022 • edited Loading

sfraczek commented Sep 27, 2022

YuanRisheng commented Sep 27, 2022 • edited Loading

sfraczek commented Sep 29, 2022

sfraczek commented Nov 28, 2022

$@sfraczek$ sfraczek commented Sep 20, 2022 •

edited

Loading

$@sfraczek$ sfraczek Sep 26, 2022

sfraczek commented Sep 26, 2022 •

edited

Loading

YuanRisheng commented Sep 27, 2022 •

edited

Loading