Use rapids-cmake parallel testing feature #12451
Merged
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
robertmaynard
added
feature request
github-actions
bot
added
ci
CMake
robertmaynard
force-pushed
the
fea/parallel_testing
branch
from
aec1146
to
d08a88c
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## branch-23.04 #12451 +/- ##
===============================================
Coverage ? 85.67%
===============================================
Files ? 155
Lines ? 24810
Branches ? 0
===============================================
Hits ? 21255
Misses ? 3555
Partials ? 0 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
(updating the branch because it will help me test something for GHAs) |
|
Moving to 23.04 |
robertmaynard
force-pushed
the
fea/parallel_testing
branch
3 times, most recently
from
76cd036
to
ad7e4a8
Compare
robertmaynard
force-pushed
the
fea/parallel_testing
branch
from
23136b1
to
fe34fd4
Compare
robertmaynard
added
3 - Ready for Review
robertmaynard
force-pushed
the
fea/parallel_testing
branch
2 times, most recently
from
66c7a0e
to
8e34774
Compare
3 tasks
robertmaynard
force-pushed
the
fea/parallel_testing
branch
5 times, most recently
from
8de0e2a
to
66cde87
Compare
robertmaynard
force-pushed
the
fea/parallel_testing
branch
from
66cde87
to
19fa78a
Compare
robertmaynard
force-pushed
the
fea/parallel_testing
branch
4 times, most recently
from
1d7cd79
to
2735e79
Compare
rapids-bot bot
pushed a commit
that referenced
this pull request
Mar 14, 2023
This PR builds on #11875 and partially addresses #11943. This PR allows us to run all tests on a precise stream (the newly introduced `cudf::test::get_default_stream()`) and then verify that all CUDA APIs end up invoked on that stream. This implements the feature required in #11943, but to apply it universally across libcudf will require the API changes that will expose streams so I plan to make those changes incrementally after this PR is merged. The preload library is now compiled twice, once to overload `cudf::get_default_stream` and once to overload `cudf::test::get_default_stream`. For now there is still some manual coordination associated with determining which one should be used with a given test, but once #12451 is merged and we start running all tests via ctest instead of direct invocation of the test executables we can start encoding this information in the CMake configuration of the tests by associating the require environment variables directly with the test executable using `set_tests_properties`. Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Robert Maynard (https://github.com/robertmaynard) - Ray Douglass (https://github.com/raydouglass) - Nghia Truong (https://github.com/ttnghia) URL: #12089
robertmaynard
force-pushed
the
fea/parallel_testing
branch
from
2735e79
to
86f501f
Compare
Converts libcudf over to use rapids-cmake new GPU aware parallel testing feature, which allows tests to run across all the GPUs on a machine without oversubscription. This will allow developers to run ctest -j<N> and ctest will figure out given the current machine how many tests it can run in parallel given the current GPU set ( currently 4 tests per GPU ).
robertmaynard
force-pushed
the
fea/parallel_testing
branch
from
86f501f
to
62c77ef
Compare
vyasr
reviewed
Mar 20, 2023
vyasr
requested changes
Mar 20, 2023
Co-authored-by: Vyas Ramasubramani <vyas.ramasubramani@gmail.com>
Co-authored-by: Vyas Ramasubramani <vyas.ramasubramani@gmail.com>
vyasr
reviewed
Mar 21, 2023
vyasr
reviewed
Mar 21, 2023
ajschmidt8
approved these changes
Mar 22, 2023
|
/merge |
3 tasks
rapids-bot bot
pushed a commit
that referenced
this pull request
Jun 9, 2023
Instead of configuring test settings with environment variables in the bash script for CI, these changes allow all that configuration to happen directly in CMake, which is more robust and also makes the test configuration portable. With these changes any execution of the tests with `ctest` will use the preload library with all the other necessary environment variables configured as well. Running the test executables directly, on the other hand, will skip stream validation but still run correctly (except for the one test that explicitly tests the stream validation preload lib itself). These changes are now possible because our CI test scripts now run ctest instead of the executables (as of #12451) and due to full support for generator expression propagation even with the parallel testing feature (as of rapidsai/rapids-cmake#410). Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - AJ Schmidt (https://github.com/ajschmidt8) - Mike Wilson (https://github.com/hyperbolic2346) URL: #13513
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Converts libcudf over to use rapids-cmake new GPU aware parallel testing feature, which allows tests to run across all the GPUs on a machine without oversubscription.
This will allow developers to run
ctest -j<N>and ctest will figure out given the current machine how many tests it can run in parallel given the current GPU set ( currently 2 tests per GPU ).Checklist