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.

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

[PHI decoupling] remove "paddle/fluid/framework/convert_utils.h" in phi #48001

Merged
merged 3 commits into from
Nov 17, 2022

Conversation

huangjiyi
Copy link
Member

PR types

Others

PR changes

Others

Describe

remove "paddle/fluid/framework/convert_utils.h" in phi

Changes:

  • paddle/fluid/framework/convert_utils.cc 中的 DataType2String 实现移动至 paddle/phi/core/utils/data_type.h,在 paddle/phi/core/utils/data_type.h 中增加一个 phi::DataTypefluidproto::VarType 的映射(参考 #47776 )。
  • 将相关文件中对 proto::VarType 及相关函数的调用替换成 phi::DataType 的形式
  • 移除或将 #include "paddle/fluid/framework/convert_utils.h" 替换为 #include "paddle/phi/core/utils/data_type.h"

@paddle-bot
Copy link

paddle-bot bot commented Nov 15, 2022

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

@huangjiyi
Copy link
Member Author

@YuanRisheng ,帮忙看下 Coverage 和 OP-benchmark 这两个 CI 的问题,我不知道怎么处理了。

@luotao1
Copy link
Contributor

luotao1 commented Nov 17, 2022

@YuanRisheng ,帮忙看下 Coverage 和 OP-benchmark 这两个 CI 的问题,我不知道怎么处理了。

@YuanRisheng 确认本 PR 可以合入后,这两个流水线可以找人豁免

@luotao1
Copy link
Contributor

luotao1 commented Nov 17, 2022

Coverage 流水线已豁免

@huangjiyi
Copy link
Member Author

@XiaoguangHu01 , request approval.

@huangjiyi
Copy link
Member Author

@ZzSean,帮忙看一下 OP-benchmark 的问题

Copy link
Contributor

@ZzSean ZzSean left a comment

Choose a reason for hiding this comment

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

LGTM for CI-OP-Benchmark

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 2f34fc7 into PaddlePaddle:develop Nov 17, 2022
@YuanRisheng
Copy link
Contributor

@huangjiyi 在你提交这个PR的过程中,convert_utils.h又有新的代码引入到了phi下,你拉一下代码搜一下就能看到。可以再提交个PR把convert_utils.h和cudnn_desc.h一起清理一下(那个新引入代码的convert_utils.h的使用和这个头文件有关)。一般情况下清理后的头文件我们会review,保证不会被再次引入,这次比较特殊,因为你俩的PR是在同一个时间段内提交的,辛苦啦

@huangjiyi
Copy link
Member Author

@huangjiyi 在你提交这个PR的过程中,convert_utils.h又有新的代码引入到了phi下,你拉一下代码搜一下就能看到。可以再提交个PR把convert_utils.h和cudnn_desc.h一起清理一下(那个新引入代码的convert_utils.h的使用和这个头文件有关)。一般情况下清理后的头文件我们会review,保证不会被再次引入,这次比较特殊,因为你俩的PR是在同一个时间段内提交的,辛苦啦

好的

@huangjiyi huangjiyi deleted the decouple_convert_utils_h branch December 9, 2022 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants