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

[Hackathon NO.71] 为 Paddle-TRT 添加 pad3d 算子 #50986

Merged
merged 30 commits into from
Mar 20, 2023

Conversation

AndSonder
Copy link
Contributor

PR types

Others

PR changes

Others

Describe

为 Paddle-TRT 添加 pad3d 算子
已通过单测,单测结果如下:

image

image

@paddle-bot
Copy link

paddle-bot bot commented Feb 27, 2023

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

Comment on lines 45 to 46
const std::vector<int> paddings =
PADDLE_GET_CONST(std::vector<int>, op_desc.GetAttr("paddings"));
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

@AndSonder AndSonder Feb 28, 2023

Choose a reason for hiding this comment

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

我参考了 pad_op.cc 的OP实现,两个API输入基本一样,但是 pad_op.cc 中并没有您上述的要求

针对value, 源代码中表明:"The pad layer of TRT only support zero."

https://github.com/AndSonder/Paddle/blob/5e8a79e1cf6d1adc7139883398b23f075fd47d79/paddle/fluid/inference/tensorrt/op_teller.cc#L1688-L1695

对于 mode 和 data_format 我也没有看到 pad_op.cc 对其进行处理,请问需要都加上吗? @zhangjun

这个题目的单测文件是你们提供,是否也需要添加上述的样例?

Copy link
Contributor

Choose a reason for hiding this comment

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

我参考了 pad_op.cc 的OP实现,两个API输入基本一样,但是 pad_op.cc 中并没有您上述的要求

针对value, 源代码中表明:"The pad layer of TRT only support zero."

https://github.com/AndSonder/Paddle/blob/5e8a79e1cf6d1adc7139883398b23f075fd47d79/paddle/fluid/inference/tensorrt/op_teller.cc#L1688-L1695

对于 mode 和 data_format 我也没有看到 pad_op.cc 对其进行处理,请问需要都加上吗? @zhangjun

这个题目的单测文件是你们提供,是否也需要添加上述的样例?

pad3d和pad有差异。 新增加OP单测需要单独添加,已有的单测文件可供参考,输入、属性完全一样的情形下可复用。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我参考了 pad_op.cc 的OP实现,两个API输入基本一样,但是 pad_op.cc 中并没有您上述的要求
针对value, 源代码中表明:"The pad layer of TRT only support zero."
https://github.com/AndSonder/Paddle/blob/5e8a79e1cf6d1adc7139883398b23f075fd47d79/paddle/fluid/inference/tensorrt/op_teller.cc#L1688-L1695
对于 mode 和 data_format 我也没有看到 pad_op.cc 对其进行处理,请问需要都加上吗? @zhangjun
这个题目的单测文件是你们提供,是否也需要添加上述的样例?

pad3d和pad有差异。 新增加OP单测需要单独添加,已有的单测文件可供参考,输入、属性完全一样的情形下可复用。

通过你们的这个单测是给出的题目要求

image

Copy link
Contributor Author

@AndSonder AndSonder Feb 28, 2023

Choose a reason for hiding this comment

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

我参考了 pad_op.cc 的OP实现,两个API输入基本一样,但是 pad_op.cc 中并没有您上述的要求
针对value, 源代码中表明:"The pad layer of TRT only support zero."
https://github.com/AndSonder/Paddle/blob/5e8a79e1cf6d1adc7139883398b23f075fd47d79/paddle/fluid/inference/tensorrt/op_teller.cc#L1688-L1695
对于 mode 和 data_format 我也没有看到 pad_op.cc 对其进行处理,请问需要都加上吗? @zhangjun
这个题目的单测文件是你们提供,是否也需要添加上述的样例?

pad3d和pad有差异。 新增加OP单测需要单独添加,已有的单测文件可供参考,输入、属性完全一样的情形下可复用。

我又仔细看了一下pad_op.cc 这个算子有很多局限的地方, 这个任务是需要实现把所有属性都考虑到吗,但是你们官方给的单测代码并没要求那么多,现在的代码已经可以完美通过你们给的单测了 @weishengying @zhangjun

Copy link
Contributor

Choose a reason for hiding this comment

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

我参考了 pad_op.cc 的OP实现,两个API输入基本一样,但是 pad_op.cc 中并没有您上述的要求
针对value, 源代码中表明:"The pad layer of TRT only support zero."
https://github.com/AndSonder/Paddle/blob/5e8a79e1cf6d1adc7139883398b23f075fd47d79/paddle/fluid/inference/tensorrt/op_teller.cc#L1688-L1695
对于 mode 和 data_format 我也没有看到 pad_op.cc 对其进行处理,请问需要都加上吗? @zhangjun
这个题目的单测文件是你们提供,是否也需要添加上述的样例?

pad3d和pad有差异。 新增加OP单测需要单独添加,已有的单测文件可供参考,输入、属性完全一样的情形下可复用。

我又仔细看了一下pad_op.cc 这个算子有很多局限的地方, 这个任务是需要实现把所有属性都考虑到吗,但是你们官方给的单测代码并没要求那么多,现在的代码已经可以完美通过你们给的单测了 @weishengying @zhangjun

单测文件只是写法示例。核心是完备的功能实现,单测需要进行新的添加,现有单测可以部分复用或者完善。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我参考了 pad_op.cc 的OP实现,两个API输入基本一样,但是 pad_op.cc 中并没有您上述的要求
针对value, 源代码中表明:"The pad layer of TRT only support zero."
https://github.com/AndSonder/Paddle/blob/5e8a79e1cf6d1adc7139883398b23f075fd47d79/paddle/fluid/inference/tensorrt/op_teller.cc#L1688-L1695
对于 mode 和 data_format 我也没有看到 pad_op.cc 对其进行处理,请问需要都加上吗? @zhangjun
这个题目的单测文件是你们提供,是否也需要添加上述的样例?

pad3d和pad有差异。 新增加OP单测需要单独添加,已有的单测文件可供参考,输入、属性完全一样的情形下可复用。

我又仔细看了一下pad_op.cc 这个算子有很多局限的地方, 这个任务是需要实现把所有属性都考虑到吗,但是你们官方给的单测代码并没要求那么多,现在的代码已经可以完美通过你们给的单测了 @weishengying @zhangjun

单测文件只是写法示例。核心是完备的功能实现,单测需要进行新的添加,现有单测可以部分复用或者完善。

了解了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

需考虑Paddings作为tensor输入情形。同时还有其他属性value、mode、data_format, 详细OP定义参考 https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/fluid/operators/pad3d_op.cc#L71-L110, 实现参考https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/phi/kernels/cpu/pad3d_kernel.cc

已添加Paddings作为tensor输入的情况,其他属性如value,mode 等添加了Tensorrt不支持的相关提示
单侧代码添加了对于Paddings为Tensor和list 情况下的测试案例

本地单侧通过

image

@AndSonder AndSonder requested a review from zhangjun March 1, 2023 04:27
Comment on lines 84 to 88
auto* layer = TRT_ENGINE_ADD_LAYER(engine_,
PaddingNd,
*const_cast<nvinfer1::ITensor*>(input),
pre_pad,
post_pad);
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.

已按照要求更改,单测添加了不同填充的mode,通过本地单测

image

@AndSonder AndSonder requested a review from zhangjun March 5, 2023 04:37
Comment on lines 1728 to 1731
#if !IS_TRT_VERSION_GE(8200)
VLOG(3) << "pad3d is not supported when TensorRT < 8.4";
return false;
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

应该是 “< 8.2”

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

Comment on lines +1737 to +1738
if (mode != "constant" && mode != "reflect" && mode != "replicate") {
VLOG(3) << "The pad3d layer of TRT only support "
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.

已修改

Comment on lines 1751 to 1758
std::vector<int64_t> shape;
auto* block = desc.Block();
if (block == nullptr) {
VLOG(3) << "The block desc is nullptr, we can't continue to analyze. "
"Developers need to check whether block_desc is passed in "
"the pass.";
return false;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

这部分可以删掉

Comment on lines 160 to 161
if len(attrs[0]['paddings']) == 6:
# for dynamic_shape
Copy link
Contributor

Choose a reason for hiding this comment

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

这里为什么要限制paddings长度为6

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已删除

value = PADDLE_GET_CONST(float, op_desc.GetAttr("value"));
}

std::string padding_mode;
Copy link
Contributor

Choose a reason for hiding this comment

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

直接给默认值constant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

Comment on lines 95 to 99
nvinfer1::ITensor* pre_pad = vectorToTensor<int>(pre_pad_v);
nvinfer1::ITensor* post_pad = vectorToTensor<int>(post_pad_v);

std::vector<int> zeros_v(inputDim, 0);
auto const zeros = vectorToTensor<int>(zeros_v);
Copy link
Contributor

Choose a reason for hiding this comment

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

有一个Add1DConstantLayer API可以直接用

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已替换

Comment on lines 176 to 179
} else if (padding_mode == "reflect") {
slice_layer->setMode(nvinfer1::SliceMode::kREFLECT);
} else if (padding_mode == "replicate") {
slice_layer->setMode(nvinfer1::SliceMode::kCLAMP);
Copy link
Contributor

Choose a reason for hiding this comment

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

这是TRT 8.2后添加的,这里Converter实现可以用宏定义包起来,参考one_hot实现。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已添加trt限制

Comment on lines 48 to 56
auto* paddings_v = scope.FindVar(op_desc.Input("Paddings")[0]);
auto* padding_t = paddings_v->GetMutable<phi::DenseTensor>();
phi::DenseTensor paddings_tensor;
paddings_tensor.Resize(padding_t->dims());
platform::CPUPlace cpu_place;
paddle::framework::TensorCopySync(
(*padding_t), cpu_place, &paddings_tensor);
auto* paddings_data =
paddings_tensor.mutable_data<int>(platform::CPUPlace());
Copy link
Contributor

Choose a reason for hiding this comment

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

有一个GetITensor的函数可以利用

Copy link
Contributor

Choose a reason for hiding this comment

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

这里写法是有问题的,动态shape下Paddings为tensor输入,这里是获取不到数据的,全部都需要使用Tensor形式操作

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这里应该是可以获取到的,我参考了 batch norm 那个op的写法,数据是从 op_desc.Input("Paddings") 里面拿到的

}
if (desc.HasAttr("mode")) {
std::string mode = PADDLE_GET_CONST(std::string, desc.GetAttr("mode"));
if (mode != "constant" || (mode != "reflect" && mode != "replicate")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

这个写法有问题, 应该是“如果全部都不等于”

Comment on lines 48 to 56
auto* paddings_v = scope.FindVar(op_desc.Input("Paddings")[0]);
auto* padding_t = paddings_v->GetMutable<phi::DenseTensor>();
phi::DenseTensor paddings_tensor;
paddings_tensor.Resize(padding_t->dims());
platform::CPUPlace cpu_place;
paddle::framework::TensorCopySync(
(*padding_t), cpu_place, &paddings_tensor);
auto* paddings_data =
paddings_tensor.mutable_data<int>(platform::CPUPlace());
Copy link
Contributor

Choose a reason for hiding this comment

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

这里写法是有问题的,动态shape下Paddings为tensor输入,这里是获取不到数据的,全部都需要使用Tensor形式操作

Comment on lines 89 to 92
for (int i = 0; i < inputDim; i += 2) {
pre_pad_v[i + 2] = paddings[pad_size - 2 - i];
post_pad_v[i + 2] = paddings[pad_size - 1 - i];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

同上面提到问题,paddings不一定能获取到

@AndSonder AndSonder requested a review from zhangjun March 14, 2023 13:41
zhangjun
zhangjun previously approved these changes Mar 17, 2023
Copy link
Contributor

@zhangjun zhangjun left a comment

Choose a reason for hiding this comment

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

LGTM

@zhangjun zhangjun self-requested a review March 19, 2023 16:48
auto* input = engine_->GetITensor(op_desc.Input("X")[0]);

nvinfer1::ITensor* paddings;
if (op_desc.HasInput("Paddings")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

需要同时满足 op_desc.Input("Paddings").size() > 0,否则会core dump

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

@@ -0,0 +1,184 @@
/* Copyright (c) 2018 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.

顺便改成2023吧

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

const framework::Scope& scope,
bool test_mode) override {
#if IS_TRT_VERSION_GE(8200)
VLOG(3) << "convert a transpose op to tensorrt tranpose layer";
Copy link
Contributor

Choose a reason for hiding this comment

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

改成pad3d

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

@zhangjun zhangjun left a comment

Choose a reason for hiding this comment

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

LGTM

@luotao1 luotao1 merged commit c36e3fd into PaddlePaddle:develop Mar 20, 2023
@AndSonder AndSonder deleted the pad3d branch April 23, 2024 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants