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

Add more tests and fix bugs for cudnn_norm_conv_test and cudnn_bn_and_relu_test #36314

Merged
merged 4 commits into from
Oct 11, 2021

Conversation

ZzSean
Copy link
Contributor

@ZzSean ZzSean commented Oct 9, 2021

PR types

Others

PR changes

Others

Describe

为cudnn_fusion_op增加了单测,更早PR见:

#35557
#35955
#36168

其中:

  • norm_conv 增加了stride!=1的测例
  • bn_add_relu 增加了融合add\relu\add+relu的测例及其反向测例

@paddle-bot-old
Copy link

paddle-bot-old bot commented Oct 9, 2021

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

Copy link
Contributor

@Xreki Xreki left a comment

Choose a reason for hiding this comment

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

LGTM. 有一处小疑问,建议后续PR确认下。

@@ -369,10 +381,25 @@ class CudnnNormConvolutionTester {
TensorCopySync(filter_grad, platform::CPUPlace(), cpu_filter_grad);
}

bool Support(const platform::CUDADeviceContext &ctx) {
Copy link
Contributor

Choose a reason for hiding this comment

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

函数名一般用IsSupport这种形式。

if ((kernel_size_ == 3) || ((kernel_size_ == 1) && (stride_ == 1))) {
return true;
}
} else if (ctx.GetComputeCapability() > 70) {
Copy link
Contributor

@Xreki Xreki Oct 11, 2021

Choose a reason for hiding this comment

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

这里可能用>=80比较好,因为还有75计算能力的GPU卡,不确定是否支持呢。文档中怎么说?另外,这个到底是跟GPU型号有关,还是跟cudnn版本有关呢?

这个检查建议挪到CudnnNormConvolution的实现里面,并且加一些注释。

@ZzSean ZzSean changed the title Add more test and fix some bug Add more test and fix some bug for cudnn_norm_conv_test and cudnn_bn_ann_relu_test Oct 11, 2021
@ZzSean ZzSean changed the title Add more test and fix some bug for cudnn_norm_conv_test and cudnn_bn_ann_relu_test Add more tests and fix bugs for cudnn_norm_conv_test and cudnn_bn_ann_relu_test Oct 11, 2021
@Xreki Xreki changed the title Add more tests and fix bugs for cudnn_norm_conv_test and cudnn_bn_ann_relu_test Add more tests and fix bugs for cudnn_norm_conv_test and cudnn_bn_and_relu_test Oct 11, 2021
@Xreki Xreki merged commit a679fcb into PaddlePaddle:develop Oct 11, 2021
@ZzSean ZzSean deleted the cudnn_fusion_test branch October 15, 2021 06:40
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.

2 participants