-
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
[ci] improve and run CUDA jobs for every commit and PR #3825
Conversation
@jameslamb |
that's great! I'd be surprised if it actually took advantage of the GPU, but at least the tests passing there is good. At some point (maybe when investigating #3776 ), we should look again at why the Dask tests fail on non-CUDA GPU builds.
I think this is totally fine! I looked at the most recent one and it took about 24 minutes to run (https://github.com/microsoft/LightGBM/actions/runs/506269302). That's quicker than other collections of jobs, so I don't think it would have too big of an impact on the peak level of activity in this repo. Because it gives us extra coverage of a part of the codebase that is very new (in terms of how many times it has been modified in its lifetime), I think it is very worth it. |
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.
I love this change! I think this will give us some new automatic test coverage that's really needed. This is going to help the pace of development a lot. Thank you for your efforts 🚀
apt-get install --no-install-recommends -y curl wget | ||
curl -sL https://cmake.org/files/v3.18/cmake-3.18.1-Linux-x86_64.sh -o cmake.sh | ||
chmod +x cmake.sh | ||
./cmake.sh --prefix=/usr/local --exclude-subdir |
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.
I'm really glad to see building CMake from source replaced with an installation from a package manager 🙌
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.
Huh, I decided that it is enough to have the most recent CMake for our other Linux jobs run inside Ubuntu 14.04 container. Thanks to default CMake from apt-get in Ubuntu 20.04 is quite fresh and passes our minimum version check!
Lines 21 to 22 in ac706e1
elseif(USE_CUDA) | |
cmake_minimum_required(VERSION 3.16) |
As a bonus, we don't need to update it in one more place regularly 🙂
@guolinke Please take a look when have time. |
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.
Thank you, LGTM
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. |
Due to
machine with NVIDIA GPU is always turned on and I don't see any reasons to not run CUDA jobs on it for every PR in this case. Running CUDA jobs on a regular basis will make us sure that no one new PR breaks CUDA source code.
CUDA jobs cannot be run in parallel but the duration of all 3 jobs I set in this PR are about 20 min. Is it OK?