-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
[CUDA] Add CUDA_ARCHITECTURES to fix CMake warnings (#3754) #4268
Conversation
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.
Thanks very much! I checked and it does seem that these changes will resolve the warnings with newer versions of CMake
.
- CUDA 11.2.2 build on latest
master
(warnings from Adjust CMake instructions of CUDA-related code for the recent CMake versions #3754): https://github.com/microsoft/LightGBM/runs/2540826588 - CUDA 11.2.2 build for this PR (no warnings): https://github.com/microsoft/LightGBM/pull/4268/checks?check_run_id=2541179844
Those CI jobs test with the latest version of CMake
, which is 3.20.2 as of today.
Get:1 https://apt.kitware.com/ubuntu focal/main amd64 cmake-data all 3.20.2-0kitware1ubuntu20.04.1 [1807 kB]
Get:2 https://apt.kitware.com/ubuntu focal/main amd64 cmake amd64 3.20.2-0kitware1ubuntu20.04.1 [10.5 MB]
Have you tested these changes against older versions of CMake
? LightGBM currently supported CMake>=3.16
for CUDA builds.
Lines 27 to 28 in c1d2dbe
elseif(USE_CUDA) | |
cmake_minimum_required(VERSION 3.16) |
I'm just leaving a "comment" review because I'd like to be sure this change doesn't break compatibility with older CMake
versions. If you haven't tested that and aren't able to easily test it, I can try in the next day or two.
Hi James, Thank you for your review. |
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.
@RobinDong Thanks a lot for picking this issue up!
I have some comments and question below:
I've updated the title of this pull request to be a bit more explicit. PR titles become changelog entries in this project (see, for example, https://github.com/microsoft/LightGBM/releases/tag/v3.2.1), and I want to make it easier for anyone looking for CUDA-related changes in the next release to find them. |
You've said you have an easy access to Linux with CUDA machine. If that's still true, could you please check this PR with CMake 3.16? I want to be sure these changes don't make old CMake panic with unrecognized target property. |
yep, definitely! I'll try soon and let you know what I find. |
Compilation seemed to succeed and I don't see any warnings! Using CUDA 10.2 and CMake 3.16.9 (the latest version in the 3.16 series, as far as I could tell from https://cmake.org/cmake/help/latest/release/3.16.html). output of nvidia-smi
I used nvidia-docker run \
--env DEBIAN_FRONTEND="noninteractive" \
-it nvidia/cuda:10.2-devel-ubuntu18.04 \
/bin/bash Then this script inside the container: testing scriptsapt-get update
apt-get install \
-y \
software-properties-common \
apt-transport-https \
curl \
git \
wget \
libcurl4-openssl-dev \
default-jdk-headless \
libssl-dev \
libxml2-dev \
libboost-dev \
libboost-system-dev \
libboost-filesystem-dev \
clinfo
# see install instructions at https://cmake.org/install/
mkdir -p /tmp/cmake-install
pushd /tmp/cmake-install
CMAKE_VERSION=3.16.9
CMAKE_TARBALL=cmake-${CMAKE_VERSION}.tar.gz
curl -OL \
https://github.com/Kitware/CMake/releases/download/v${CMAKE_VERSION}/${CMAKE_TARBALL}
tar -xvzf ${CMAKE_TARBALL}
rm -rf ./${CMAKE_TARBALL}
cd cmake-${CMAKE_VERSION}
./bootstrap
make
make install
popd
# check CMake version
cmake --version
# get LightGBM from PR 4268
git clone --recursive https://github.com/RobinDong/LightGBM.git --branch master
cd LightGBM
mkdir ./build
cd ./build
cmake \
-DCMAKE_CUDA_COMPILER=/usr/local/cuda/bin/nvcc \
-DUSE_CUDA=ON \
..
make _lightgbm -j4 See the logs below. logs from 'cmake ..'ran the following cmake \
-DCMAKE_CUDA_COMPILER=/usr/local/cuda/bin/nvcc \
-DUSE_CUDA=ON \
.. which resulted in these logs
logs from 'make _lightgbm'ran the following make _lightgbm -j2 which resulted in the following logs
Now that I have this script working, it would be fairly easy and quick to test other CMake versions, so please let me know if there are other versions you'd like me to check. |
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.
@jameslamb Thank you very much for verifying with CMake 3.16!
@RobinDong Many thanks for your great help! We will really appreciate your further contributions to CMake and CUDA things.
@RobinDong Please update to the latest |
This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this. |
Set property
CUDA_ARCHITECTURES
would fix the warnings reported by CMake 3.20.x