This repository has been archived by the owner on Mar 21, 2024. It is now read-only.
Overhaul CUB test suite. #403
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
alliepiper
added
P0: must have
alliepiper
added a commit
to alliepiper/thrust
that referenced
this pull request
Nov 9, 2021
|
gpuCI: NVIDIA/thrust#1563 |
This was referenced Nov 9, 2021
alliepiper
force-pushed
the
gh.399/test_overhaul
branch
7 times, most recently
from
75812e3
to
450bf13
Compare
alliepiper
force-pushed
the
gh.399/test_overhaul
branch
2 times, most recently
from
83d0520
to
088f6d3
Compare
alliepiper
force-pushed
the
gh.399/test_overhaul
branch
from
088f6d3
to
451dee0
Compare
alliepiper
force-pushed
the
gh.399/test_overhaul
branch
2 times, most recently
from
9d34e77
to
8bb6e90
Compare
|
DVS CL: 30772865 |
alliepiper
added
testing: internal ci in progress
alliepiper
added
the
testing: internal ci passed
gevtushenko
approved these changes
Dec 15, 2021
Issue #399 reported that we were missing several test cases in CUB that were ifdef'd out. This patch enables most of those tests, though CDP tests are not added here. Some other deficiencies were addressed as they were noticed, for instance, adding value_types other than unsigned char to test_block_histogram. The way we split up tests into "BENCHMARK", "MINIMAL", and "THOROUGH" variants wasn't well suited for regression testing, as a lot of redundant code paths were generated between the various test executables. These have been removed, leaving only the "THOROUGH" tests, which should capture all test cases. Benchmarks should go into the new `thrust_benchmark` project. Some tests also took an excessively long time to build, especially after enabling the missing test cases from #399. This patch adds a new mechanism that allows a test to include a comment such as: ``` // %PARAM% TEST_FOO foo 0:1:2 // %PARAM% TEST_BAR bar 4:8 ``` CMake will parse these out, and generate multiple test executables for each combination of parameters, e.g: ``` cub.test.baz.foo_0.bar_4 -DTEST_FOO=0 -DTEST_BAR=4 cub.test.baz.foo_0.bar_8 -DTEST_FOO=0 -DTEST_BAR=8 cub.test.baz.foo_1.bar_4 -DTEST_FOO=1 -DTEST_BAR=4 cub.test.baz.foo_1.bar_8 -DTEST_FOO=1 -DTEST_BAR=8 cub.test.baz.foo_2.bar_4 -DTEST_FOO=2 -DTEST_BAR=4 cub.test.baz.foo_2.bar_8 -DTEST_FOO=2 -DTEST_BAR=8 ``` This can be used to quickly split up problematically large tests. See the note at the top of cub/test/CMakeLists.txt for more details. The PrintNinjaBuildTimes.cmake file from Thrust was used to identify tests that needed to be split. Several tests were testing Thrust APIs. This isn't necessary, as Thrust has it's own test suite. These tests have been removed. This isn't needed for regression testing and has been removed. Some of the other command line options could also be removed now that benchmarking isn't handled by these regression tests, but this is a start. Extended testing revealed that the cub::BlockHistogram algorithm's behavior is undefined when input values are outside of [0, BINS). Added this info to the algorithm docs. * test_device_histogram from 15m -> 35s. * test_device_run_length_encode from 7m -> 3s. * test_device_scan tests from <3m -> <4s. # Conflicts: # test/test_device_radix_sort.cu # Conflicts: # test/test_device_radix_sort.cu
alliepiper
force-pushed
the
gh.399/test_overhaul
branch
from
8bb6e90
to
2f45014
Compare
|
DVS CL: 30781720 gpuCI: NVIDIA/thrust#1564 |
alliepiper
added
testing: gpuCI in progress
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.
Missing test cases
Issue NVIDIA/cccl#881 reported that we were missing several test cases in CUB
that were ifdef'd out. This patch enables most of those tests, though
CDP tests are not added here.
Some other deficiencies were addressed as they were noticed, for instance,
adding value_types other than unsigned char to test_block_histogram.
After these changes, the test suite build took >12 hours to build...prompting the rest of the changes in this PR, which lowered the serial build time to about 1h20m.
Removed MINIMAL and BENCHMARK variants
The way we split up tests into "BENCHMARK", "MINIMAL", and "THOROUGH"
variants wasn't well suited for regression testing, as a lot of
redundant code paths were generated between the various test
executables. These have been removed, leaving only the "THOROUGH" tests,
which should capture all test cases.
Benchmarks should go into the new
thrust_benchmarkproject.New %PARAM% mechanism
This patch adds a new mechanism that allows a test to include a comment such as:
CMake will parse these out and generate multiple test executables for
each combination of parameters, e.g:
This can be used to quickly split up problematically large tests. See
the new
cub/test/README.mdfile for more details. ThePrintNinjaBuildTimes.cmake file from Thrust was used to identify
tests that needed to be split.
Remove non-CUB test code
Several tests were testing Thrust APIs. This isn't necessary, as Thrust
has it's own test suite. These tests have been removed.
Removed
g_repeatoptionsThis isn't needed for regression testing and has been removed. Some
of the other command line options could also be removed now that
benchmarking isn't handled by these regression tests, but this is a start.