This repository has been archived by the owner on Nov 17, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
leezu
force-pushed
the
cmakefixcudnn
branch
2 times, most recently
from
December 9, 2019 07:34
7a0b009
to
19af332
Compare
marcoabreu
suggested changes
Dec 9, 2019
marcoabreu
approved these changes
Dec 9, 2019
leezu
force-pushed
the
cmakefixcudnn
branch
2 times, most recently
from
December 9, 2019 10:48
a21e4a9
to
5df2368
Compare
- Use FindCUDNN.cmake instead of the previous macro - Previous macro did not detect CUDNN correctly on my system (Deep Learning AMI Ubuntu 18.04) - We now explicitly fail if the user does not provide CUDNN and hasn't manually set -DUSE_CUDNN=0
If the lazy consensus for bumping cmake minimum version requirement on the mailing list passes, I'll open another PR tomorrow removing all custom Cuda handling logic. Only the |
marcoabreu
reviewed
Dec 9, 2019
yajiedesign
approved these changes
Dec 10, 2019
leezu
commented
Dec 10, 2019
@@ -502,13 +502,15 @@ add_subdirectory(${GTEST_ROOT}) | |||
find_package(GTest REQUIRED) | |||
|
|||
# cudnn detection | |||
if(USE_CUDNN AND USE_CUDA) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This AND USE_CUDA
should not have been removed inside this PR. It assumes cmake_dependent_option(USE_CUDNN "Build with cudnn support" ON "USE_CUDA" OFF)
from #17018 is used. Unfortunately our tests don't test USE_CUDA=0
on a machine with CUDNN available, so the passed.
@yajiedesign I suggest either to either merge #17018 quickly or I'll create another PR to add back the AND USE_CUDA
yajiedesign
pushed a commit
to yajiedesign/mxnet
that referenced
this pull request
Jan 5, 2020
…ranch Fix CUDNN detection for CMake build (apache#17019) Replace mxnet_option macro with standard CMAKE_DEPENDENT_OPTION (apache#17018) Switch to modern CMake CUDA handling (apache#17031) Introduce unified MXNET_CUDA_ARCH option to specify cuda architectures. Previously cuda architecture setting was partially broken and different options were applied to different parts of the build (CUDA_ARCH_NAME CUDA_ARCH_BIN CUDA_ARCH_PTX and CUDA_ARCH_LIST). Include FindCUDAToolkit from CMake 3.17, which replaces the deprecated FindCUDA functionality for finding the cuda toolkit include directories and libraries. Workaround for DLL size limitation on Windows (apache#16980) * change windows build system. add gen_warp cpp version add add_custom_command to run warp_gen add download cmake add option change option add dynamic read mxnet dll
ptrendx
pushed a commit
that referenced
this pull request
Jan 5, 2020
Fix CUDNN detection for CMake build (#17019) Replace mxnet_option macro with standard CMAKE_DEPENDENT_OPTION (#17018) Switch to modern CMake CUDA handling (#17031) Introduce unified MXNET_CUDA_ARCH option to specify cuda architectures. Previously cuda architecture setting was partially broken and different options were applied to different parts of the build (CUDA_ARCH_NAME CUDA_ARCH_BIN CUDA_ARCH_PTX and CUDA_ARCH_LIST). Include FindCUDAToolkit from CMake 3.17, which replaces the deprecated FindCUDA functionality for finding the cuda toolkit include directories and libraries. Workaround for DLL size limitation on Windows (#16980) * change windows build system. add gen_warp cpp version add add_custom_command to run warp_gen add download cmake add option change option add dynamic read mxnet dll Co-authored-by: Leonard Lausen <leonard@lausen.nl>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Ubuntu 18.04) and did not generate any warnings that cudnn could not be found.
With FindCUDNN.cmake introduced here, based on https://cmake.org/cmake/help/latest/module/FindPackageHandleStandardArgs.html, we will use the "standard" outputting methods to signify if cuDNN is found or missing.
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments
CC @szha @yajiedesign @junrushao1994