Skip to content

[PHI]add phi tensor vector array api from fluid #49885

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
Merged
merged 1 commit into from
Jan 30, 2023
Merged

[PHI]add phi tensor vector array api from fluid #49885

merged 1 commit into from
Jan 30, 2023

Conversation

@engineer1109
Copy link
Contributor

@engineer1109 engineer1109 commented Jan 17, 2023 •
edited
Loading

PR types

Function optimization

PR changes

OPs

Describe

PHI算子解藕代码
将TensorFromVector和TensorFromArray和TensorToVector从fluild的tensor_utils
移植到paddle/phi/core/tensor_utils.cc

对paddle/phi/kernels/assign_kernel.cc实现fluid头文件解藕
全部的PHI算子相关部分解藕

paddle/fluid/framework/tensor_util.h
有一个宏拼写错误 PADDLE_WITH_CUSTOM_DEICE 改为PADDLE_WITH_CUSTOM_DEVICE

@paddle-bot
Copy link

paddle-bot bot commented Jan 17, 2023

你的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.

@paddle-bot paddle-bot bot added contributor External developers status: proposed labels Jan 17, 2023
@engineer1109 engineer1109 force-pushed the assignphi branch 3 times, most recently from 29b91fe to d25e126 Compare January 17, 2023 07:35
YuanRisheng
YuanRisheng reviewed Jan 18, 2023
View reviewed changes
Copy link
Contributor

@YuanRisheng YuanRisheng left a comment

Choose a reason for hiding this comment

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

可以把phi下其他用到这俩个函数的地方都替换成phi命名空间

paddle/phi/core/tensor_utils.cc Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

这个宏下边包含一些paddle::platform命名空间下的内容，会增加phi头文件解耦难度，并且npu相关的内容可以不用包含在phi下，这个宏里的实现可以先删掉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已经删除

paddle/phi/core/tensor_utils.cc Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

mlu宏里的内容也可以删掉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已经删除

paddle/phi/core/tensor_utils.cc Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已经删除

paddle/phi/core/tensor_utils.cc Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已经删除

@engineer1109 engineer1109 force-pushed the assignphi branch 9 times, most recently from bcdbea4 to 5b88b56 Compare January 18, 2023 06:51
@engineer1109
Copy link
Contributor Author

@YuanRisheng 已经修改，并把TensorToVector也加入了。
PR-CI-APPROVAL 因为修改文件数，无法通过。
PR-CI-Coverage 可能因为一些代码没有跑到，无法通过。
等待CI完成

@engineer1109
Copy link
Contributor Author

@YuanRisheng CI完成了。
PR-CI-APPROVAL 因为修改文件数超过20，无法通过。
PR-CI-Coverage 无法覆盖CUSTOM_DEVICE代码和一些从未测试的算子，无法通过。
PR-CI-OP-benchmark 因为determinant_0变慢了一点无法通过，没有看出啥问题

YuanRisheng
YuanRisheng reviewed Jan 18, 2023
View reviewed changes
paddle/phi/kernels/assign_kernel.cc Outdated
Copy link
Contributor

@YuanRisheng YuanRisheng Jan 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

抱歉，第一次review的时候这里没注意到，这里应该是false，TensorCopy对应的是异步拷贝，对应phi下参数传递false，麻烦再修改一下

@YuanRisheng
Copy link
Contributor

@YuanRisheng CI完成了。
PR-CI-APPROVAL 因为修改文件数超过20，无法通过。
PR-CI-Coverage 无法覆盖CUSTOM_DEVICE代码和一些从未测试的算子，无法通过。
PR-CI-OP-benchmark 因为determinant_0变慢了一点无法通过，没有看出啥问题

Approval等待approve后会通过，coverage及benchmark，等copy修改完ci跑完后，麻烦@luotao1看一下是否能豁免

@engineer1109
add phi tensor vector array api from fluid
fd7e960 
replace all TensorFromVector & TensorToVector

AssignKernel async copy
@engineer1109
Copy link
Contributor Author

PR-CI-OP-benchmark又通过了。 Approval和coverage需要处理
@luotao1 @YuanRisheng

@luotao1
Copy link
Contributor

luotao1 commented Jan 30, 2023

Coverage流水线已豁免

XiaoguangHu01
XiaoguangHu01 approved these changes Jan 30, 2023
View reviewed changes
Copy link
Contributor

@XiaoguangHu01 XiaoguangHu01 left a comment

Choose a reason for hiding this comment

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

LGTM

@luotao1 luotao1 merged commit 094e3b8 into PaddlePaddle:develop Jan 30, 2023
@engineer1109
Copy link
Contributor Author

#47615 解藕paddle/fluid/framework/tensor_util.h

@YuanRisheng YuanRisheng mentioned this pull request Jan 30, 2023
Closed
@engineer1109 engineer1109 mentioned this pull request Feb 1, 2023
Merged
pangengzheng pushed a commit to pangengzheng/Paddle that referenced this pull request Feb 2, 2023
@engineer1109 @pangengzheng
add phi tensor vector array api from fluid (PaddlePaddle#49885)
eb40e7d 
replace all TensorFromVector & TensorToVector

AssignKernel async copy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@YuanRisheng YuanRisheng YuanRisheng approved these changes

@XiaoguangHu01 XiaoguangHu01 XiaoguangHu01 approved these changes

Assignees

@luotao1 luotao1

@YuanRisheng YuanRisheng

Labels

contributor External developers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@engineer1109 @YuanRisheng @luotao1 @XiaoguangHu01