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

Move psroi_pool OP to phi #40353

Merged
merged 2 commits into from
Mar 11, 2022
Merged

Conversation

From00
Copy link
Contributor

@From00 From00 commented Mar 9, 2022

PR types

Function optimization

PR changes

OPs

Describe

Move psroi_pool OP to phi.

@paddle-bot-old
Copy link

paddle-bot-old bot commented Mar 9, 2022

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

Comment on lines +138 to +139
kernel->InputAt(2).SetDataType(
paddle::experimental::CppTypeToDataType<int>::Type());
Copy link
Contributor

Choose a reason for hiding this comment

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

这是不是可以直接写成DataType::INT32?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

在许多情况下两者应该是一样的。但考虑到原kernel里直接使用的int类型并不是一种跨平台安全的类型,int和INT32类型不等价的情况是允许的,所以这里还是使用CppTypeToDataType让编译器做类型转换更安全一些。

PD_REGISTER_KERNEL(
psroi_pool_grad, GPU, ALL_LAYOUT, phi::PsroiPoolGradKernel, float, double) {
kernel->InputAt(2).SetDataType(
paddle::experimental::CppTypeToDataType<int>::Type());
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/core/device_context.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

这里应该没有使用device_context.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.

done, thx

#pragma once

#include "paddle/phi/core/dense_tensor.h"
#include "paddle/phi/core/device_context.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

}

DenseTensor rois_batch_id_list_gpu;
paddle::framework::TensorCopy(
Copy link
Contributor

Choose a reason for hiding this comment

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

可以看下是否可以使用phi下的copy来替代

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, thx

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

@From00 From00 merged commit c0e2923 into PaddlePaddle:develop Mar 11, 2022
@From00 From00 deleted the move-psroi-pool-op-to-phi branch April 4, 2022 12:28
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.

6 participants