Skip to content

Conversation

@Aganlengzi
Copy link
Contributor

@Aganlengzi Aganlengzi commented Feb 23, 2022

PR types

Others

PR changes

Others

Describe

Migrate increment addmm multinomial and cholesky kernel to phi.

TODO: migrate InferShape funcs. move MultinomialFunctor to paddle/phi/kernels/funcs.

DenseTensor* out);

template <typename T>
void MultinomialFunctor(int64_t* out_data, const T* in_data,
Copy link
Contributor

Choose a reason for hiding this comment

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

MultinomialFunctor 不要放到kernel 声明的头文件中,可以放到paddle/phi/kernels/funcs 下

Copy link
Contributor Author

Choose a reason for hiding this comment

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

多谢,我在迁InferShape函数的PR中修改.

Copy link
Contributor

Choose a reason for hiding this comment

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

可以下个PR 再移动

Copy link
Contributor

@MingMingShangTian MingMingShangTian left a comment

Choose a reason for hiding this comment

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

LGTM

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 for overall,细节后续PR可以完善下

@@ -0,0 +1,28 @@
/* Copyright (c) 2021 PaddlePaddle Authors. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

2022?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

多谢


namespace phi {

KernelSignature AddmmOpArgumentMapping(const ArgumentMappingContext& ctx) {
Copy link
Contributor

Choose a reason for hiding this comment

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

这个和opmaker的顺序一致,后面PR应该可以删除

Copy link
Contributor Author

Choose a reason for hiding this comment

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

了解,InferShape迁移中会移除。


namespace phi {

KernelSignature CholeskyOpArgumentMapping(const ArgumentMappingContext& ctx) {
Copy link
Contributor

@chenwhql chenwhql Feb 24, 2022

Choose a reason for hiding this comment

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

同上,应该也不需要写,默认生成的就可以用,这些ArgumentMapping函数之所以放到compat目录下,就是希望将来可以移除的,所以我们不写不必要的

Copy link
Contributor Author

Choose a reason for hiding this comment

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

了解,InferShape迁移中会移除。

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

Labels

None yet

4 participants