Skip to content

[Phi] mv kernel #39861

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

Merged
merged 1 commit into from
Feb 25, 2022
Merged

Conversation

windstamp
Copy link
Contributor

@windstamp windstamp commented Feb 23, 2022

PR types

New features

PR changes

OPs

Describe

[Phi] mv kernel

@paddle-bot-old
Copy link

Hi, It's a test PR, it will not trigger CI. If you want to trigger CI, please remove notest in your commit message.

@windstamp windstamp force-pushed the npu_dev_phi_20220223 branch 2 times, most recently from 1c1a3df to f168115 Compare February 24, 2022 02:56
@windstamp windstamp force-pushed the npu_dev_phi_20220223 branch from f168115 to ea81b5c Compare February 24, 2022 06:38
@windstamp windstamp force-pushed the npu_dev_phi_20220223 branch from ea81b5c to 9ef3ab6 Compare February 24, 2022 11:07
@windstamp windstamp force-pushed the npu_dev_phi_20220223 branch from 9ef3ab6 to 588929a Compare February 24, 2022 11:39
@windstamp windstamp changed the title [Phi] mv [Phi] mv kernel Feb 25, 2022
} // namespace phi

PD_REGISTER_KERNEL(mv_grad, CPU, ALL_LAYOUT, phi::MvGradKernel, float, double) {
}
Copy link
Contributor

Choose a reason for hiding this comment

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

这个pre-commit处理的换行吗,有点别扭,不如直接在kernel name那里换个行

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是的,pre-commit 某种情况下会这样自动更新一下,后来我又改回来了,但是又会导致 CI (应该是 PR-CI-Static-Check) 失败,必须这样才能通过。

你们是否可以和 PR-CI-Static-Check 确认一下,这种看着确实有点别扭。


namespace phi {

KernelSignature MvOpArgumentMapping(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.

这个op挺标准的,可以不用写这个函数,建议后续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.

好的,后续 PR 修复


namespace phi {

KernelSignature MvOpArgumentMapping(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.

这里前向的Mapping映射应该可以不写,使用默认op_proto映射即可,可以在后续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.

好的,后续 PR 修复

@windstamp windstamp merged commit 2553af4 into PaddlePaddle:develop Feb 25, 2022
@windstamp windstamp deleted the npu_dev_phi_20220223 branch February 25, 2022 04:32
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.

3 participants