Skip to content

Conversation

@YuanRisheng
Copy link
Contributor

PR types

Others

PR changes

Others

Describe

Add alias kernel name for compatible with old kernel name and modify the kernel name in pten. This will make pten kernel's name more clear.

@paddle-bot-old
Copy link

paddle-bot-old bot commented Dec 6, 2021

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


namespace pten {

// the map between kernel name in fluid and pten
Copy link
Contributor

Choose a reason for hiding this comment

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

这里再加些注释,说明一下key,value 是什么含义吧

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines 25 to 35
{"reshape2", "reshape"},
{"fill_any_like", "full_like"},
{"fill_constant", "full"},
{"matmul_v2", "matmul"},
{"flatten_contiguous_range", "flatten"},
{"reduce_mean", "mean"},
{"reduce_sum", "sum"},
{"elementwise_add", "add"},
{"elementwise_sub", "subtract"},
{"elementwise_mul", "muliply"},
{"elementwise_div", "divide"},
Copy link
Contributor

Choose a reason for hiding this comment

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

这里是不可以按照一个统一的规则排下序,后面迁移的kernel多了找起来也方便

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}
}

std::string TransToPtenKernelName(const std::string& fluid_op_name) {
Copy link
Contributor

Choose a reason for hiding this comment

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

可以返回const std::string&

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

using DataType = paddle::experimental::DataType;
using DataLayout = paddle::experimental::DataLayout;

std::string TransToPtenKernelName(const std::string& fluid_op_name);
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

const auto* op_proto = pair.second.proto_;
if (pten::KernelFactory::Instance().HasCompatiblePtenKernel(op_type)) {
KernelArgsNameMakerByOpProto maker(op_proto);
VLOG(10) << "Register kernel signature for " << op_type;
Copy link
Contributor

Choose a reason for hiding this comment

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

这里的op_type 是不是也要pten::TransToPtenKernelName(op_type)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

不需要

PT_REGISTER_MODULE(CreationCPU);

PT_REGISTER_KERNEL("fill_any_like",
PT_REGISTER_KERNEL("full_like",
Copy link
Contributor

Choose a reason for hiding this comment

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

这样直接改会导致pten kernel在运行时都没有被选到吧,兼容态是根据这个名字来搜索是否执行pten kernel的

Copy link
Contributor Author

Choose a reason for hiding this comment

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

会选到


// the map between kernel name in fluid and pten
const std::unordered_map<std::string, std::string> kernel_alias_name_map = {
{"reshape2", "reshape"},
Copy link
Contributor

Choose a reason for hiding this comment

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

这个会导致咱们让大家迁移的时候,还需要额外关注这个文件的更新和映射,能否在注册时一起声明下这个name

Copy link
Contributor

Choose a reason for hiding this comment

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

如果用这种的话,建议这个map单独放到一个文件中管理

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@chenwhql chenwhql left a comment

Choose a reason for hiding this comment

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

LGTM

@chenwhql chenwhql merged commit ff6507d into PaddlePaddle:develop Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants