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

[API2.0] add op for cudnn version query test=develop #26180

Merged
merged 9 commits into from
Aug 16, 2020
Merged

[API2.0] add op for cudnn version query test=develop #26180

merged 9 commits into from
Aug 16, 2020

Conversation

wangchaochaohu
Copy link
Contributor

@wangchaochaohu wangchaochaohu commented Aug 12, 2020

PR types

New features

PR changes

APIs

Describe

Add cudnn version query: def get_cudnn_version
this api return the version of cudnn.

@paddle-bot-old
Copy link

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

@paddle-bot-old
Copy link

paddle-bot-old bot commented Aug 12, 2020

✅ This PR's description meets the template requirements!
Please wait for other CI results.

@wangchaochaohu wangchaochaohu changed the title refine the code test=develop [API2.0] add op for device query test=develop Aug 12, 2020
python/paddle/device.py Outdated Show resolved Hide resolved
paddle/fluid/platform/CMakeLists.txt Show resolved Hide resolved
self.assertEqual((cudnn_version is None), True)

def test_cudnn(self):
cudnn_version = paddle.cudnn_version()
Copy link
Contributor

Choose a reason for hiding this comment

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

incomplete test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wo don't know ci machine cudnn version

Copy link
Contributor

Choose a reason for hiding this comment

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

at least check if it returns an integer?

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

]


def cudnn_version():
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to get_cudnn_version() to signify that it is a function not a variable?

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

self.assertEqual((cudnn_version is None), True)

def test_cudnn(self):
cudnn_version = paddle.cudnn_version()
Copy link
Contributor

Choose a reason for hiding this comment

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

at least check if it returns an integer?



class TestCudnnVersion(unittest.TestCase):
def test_no_cudnn(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

this can be merged into the test below, since you can not test both case in one go.

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

@willthefrog willthefrog left a comment

Choose a reason for hiding this comment

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

maybe give some explanation of the return value, something like 7600 -> 7.6? otherwise LGTM.

@@ -73,6 +74,10 @@ int GetCUDADeviceCount() {
return dev_cnt;
}

/* Here is a very simple CUDA “pro tip”: cudaDeviceGetAttribute() is a much
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this comment here? looks like that cudaDeviceGetAttribute is not used anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comment is for GetCUDAComputeCapability, I change it to right position by the way

]


def get_cudnn_version():
Copy link
Contributor

Choose a reason for hiding this comment

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

the return value should be cached, as it will never change and this function may be called on each step for each OP that use it in dygraph 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.

Done

willthefrog
willthefrog previously approved these changes Aug 14, 2020
Copy link
Contributor

@willthefrog willthefrog left a comment

Choose a reason for hiding this comment

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

LGTM

heavengate
heavengate previously approved these changes Aug 15, 2020
@wangchaochaohu wangchaochaohu changed the title [API2.0] add op for device query test=develop [API2.0] add op for cudnn version query test=develop Aug 16, 2020
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.

4 participants