-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
[PHI decoupling] remove "paddle/fluid/framework/convert_utils.h" in phi #48001
Conversation
你的PR提交成功,感谢你对开源项目的贡献! |
@YuanRisheng ,帮忙看下 Coverage 和 OP-benchmark 这两个 CI 的问题,我不知道怎么处理了。 |
等 @YuanRisheng 确认本 PR 可以合入后,这两个流水线可以找人豁免 |
Coverage 流水线已豁免 |
@XiaoguangHu01 , request approval. |
@ZzSean,帮忙看一下 OP-benchmark 的问题 |
There was a problem hiding this 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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@huangjiyi 在你提交这个PR的过程中,convert_utils.h又有新的代码引入到了phi下,你拉一下代码搜一下就能看到。可以再提交个PR把convert_utils.h和cudnn_desc.h一起清理一下(那个新引入代码的convert_utils.h的使用和这个头文件有关)。一般情况下清理后的头文件我们会review,保证不会被再次引入,这次比较特殊,因为你俩的PR是在同一个时间段内提交的,辛苦啦 |
好的 |
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::DataType
到fluid
中proto::VarType
的映射(参考 #47776 )。proto::VarType
及相关函数的调用替换成phi::DataType
的形式#include "paddle/fluid/framework/convert_utils.h"
替换为#include "paddle/phi/core/utils/data_type.h"