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] Migrate multiplex, qr, tril_triu op kernel to phi #40007

Merged
merged 16 commits into from
Mar 16, 2022

Conversation

Caozhou1995
Copy link
Contributor

PR types

Others

PR changes

Others

Describe

This pr migrate kernel and grad kernel on the cpu and gpu of multiplex, cpu kernel of qr, kernel and grad kernel on the cpu and gpu of tril_triu to phi.

@paddle-bot-old
Copy link

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


namespace phi {

static inline std::tuple<bool, bool> _parse_qr_mode(const std::string& mode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

_parse_qr_mode 这个函数命名不合规范,改为驼峰式

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx, done

@@ -16,17 +16,16 @@

#include "paddle/phi/kernels/triangular_solve_grad_kernel.h"

#include "paddle/fluid/platform/for_range.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

使用phi下的这个头文件 paddle/phi/kernels/funcs/for_range.h

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx, done


#include "paddle/phi/kernels/tril_triu_grad_kernel.h"

#include "paddle/fluid/platform/for_range.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

同上


#include "paddle/phi/kernels/tril_triu_kernel.h"

#include "paddle/fluid/platform/for_range.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

#pragma once

#include "paddle/phi/core/dense_tensor.h"
#include "paddle/phi/kernels/funcs/eigen/common.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

这个头文件没用到

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx, already removed

@@ -145,8 +145,6 @@ REGISTER_OPERATOR(qr, ops::QrOp, ops::QrOpMaker,

REGISTER_OPERATOR(qr_grad, ops::QrGradOp);

REGISTER_OP_CPU_KERNEL(qr, ops::QrCPUKernel<float>, ops::QrCPUKernel<double>);

REGISTER_OP_CPU_KERNEL(
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.

是的,因为有依赖,后续由威行或威行解完依赖我再迁移


#include "paddle/phi/kernels/multiplex_kernel.h"

#include "paddle/phi/api/lib/utils/tensor_utils.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

这个头文件有用到吗

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已经挪走

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

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 PADDLE_ENFORCE


template <typename T, typename Context>
void MultiplexKernel(const Context& ctx,
const std::vector<DenseTensor*>& ins,
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::vector<const DenseTensor*>& ins

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx, done

@chenwhql chenwhql merged commit dce87e3 into PaddlePaddle:develop Mar 16, 2022
liqitong-a pushed a commit to liqitong-a/Paddle that referenced this pull request Mar 17, 2022
…#40007)

* migrate multiplex op kernel

* migrate qr cpu kernel

* migrate tril_triu op kernel

* fix multiplex kernel

* add kernel sig

* fix dependence and bug

* fix multiplex error

* fix npu include error

* fix conflict

* fix conflict and delete tril_triu

* fix date and multiplex input

* adapt header file order

* fix header file include

* fix conflict

* delete cholesky_solve_op.h

* delete triangular_solve_op.h
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.

5 participants