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

[ROCM] added a cudnn switch of conv2d for rocm platform #31836

Merged
merged 1 commit into from
Mar 29, 2021

Conversation

ronny1996
Copy link
Contributor

PR types

Others

PR changes

Others

Describe

added a cudnn switch of conv2d for rocm platform

@paddle-bot-old
Copy link

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

@@ -629,6 +633,11 @@ def __init__(self,
weight_attr=None,
bias_attr=None,
data_format="NCHW"):
use_cudnn = True
Copy link
Contributor

Choose a reason for hiding this comment

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

这里写成类似这样的形式
use_cudnn = True if (core.is_compiled_with_cuda() and
cudnn_version is not None) else False

@@ -153,6 +156,7 @@ def _get_default_param_initializer():
out_channels % in_channels == 0):
self._op_type = 'depthwise_conv2d'
self._use_cudnn = False
self._use_cudnn = self._use_cudnn and use_cudnn
Copy link
Contributor

Choose a reason for hiding this comment

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

加一行注释,解释一下这里这么修改的原因,例如
// add environment of ROCM_CONV2D_DISABLE_CUDNN to disable use_cudnn in Conv2D
// If use_cudnn is not defined, default value is True, then self._use_cudnn = self._use_cudnn

qili93
qili93 previously approved these changes Mar 24, 2021
Copy link
Contributor

@qili93 qili93 left a comment

Choose a reason for hiding this comment

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

LGTM, api test will be covered by CPU CI and CUDA CI

@@ -1498,6 +1498,9 @@ def conv2d(input,
conv2d = paddle.static.nn.conv2d(input=data, num_filters=2, filter_size=3, act="relu")
print(conv2d.shape) # [-1, 2, 30, 30]
"""
use_cudnn = False if (
core.is_compiled_with_rocm() and
bool(os.getenv("ROCM_CONV2D_DISABLE_CUDNN"))) else use_cudnn
Copy link
Contributor

Choose a reason for hiding this comment

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

If ROCM_CONV2D_DISABLE_CUDNN is not a built-in variable in ROCM but created by Paddle, I suggest following the naming rule like other environments like "FLAGS_xxx_xxx".

Copy link
Contributor

Choose a reason for hiding this comment

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

Plus, it's better to get the value by fluid.get_flags instead of os.getenv

Copy link
Contributor Author

Choose a reason for hiding this comment

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

update

qili93
qili93 previously approved these changes Mar 26, 2021
Copy link
Contributor

@qili93 qili93 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

@zhiqiu zhiqiu 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.fluid.layers.conv2d

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