Update to Thrust 1.17.0 #11437
Merged
Update to Thrust 1.17.0 #11437
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
github-actions
bot
added
CMake
bdice
added
CMake
bdice
commented
Aug 2, 2022
Codecov Report
@@ Coverage Diff @@
## branch-22.10 #11437 +/- ##
===============================================
Coverage ? 86.48%
===============================================
Files ? 144
Lines ? 22850
Branches ? 0
===============================================
Hits ? 19761
Misses ? 3089
Partials ? 0 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
Can we investigate the increased compile times here? The 2 mixed-join source files used to be the longest by far. |
|
Now Previously, by default we never had |
|
+1 on this update, having |
3 tasks
rapids-bot bot
pushed a commit
that referenced
this pull request
Aug 4, 2022
Thrust 1.16 removed internal header inclusions that libcudf relied on. This PR adds missing `#include`s that were found automatically by a script I wrote. See notes on #10489. This was previously applied in #10489 but the script became more sophisticated (and libcudf has changed) since I last applied it, so more missing `#include`s were found. Required for #11437 to upgrade to Thrust 1.17. This change has been separated from #11437 to minimize that PR's diff. Some additional changes will be needed on that PR but we don't want to hold off on fixing these includes, as recommended by @davidwendt. Authors: - Bradley Dice (https://github.com/bdice) Approvers: - Karthikeyan (https://github.com/karthikeyann) - Nghia Truong (https://github.com/ttnghia) - Robert Maynard (https://github.com/robertmaynard) URL: #11457
3 tasks
|
The build metrics report for 18bd5e8 appears to indicate that the patch worked. The total libcudf.so size dropped from 461 MB to 407 MB, too! |
|
@GregoryKimball Can we run benchmark on this open PR before merging? So we won't have to reverse if something happens? |
rapids-bot bot
pushed a commit
that referenced
this pull request
Aug 5, 2022
…unctor (#11482) Refactored the `group_nunique.cu` source to use the `nullate::DYNAMIC` for the equal operator and the unique-iterator. This improves the compile time by almost 2x without much change to performance by reducing the number of calls to `thrust::reduce_by_key`. Found while investigating compile issues for #11437 Authors: - David Wendt (https://github.com/davidwendt) Approvers: - Bradley Dice (https://github.com/bdice) - Mike Wilson (https://github.com/hyperbolic2346) - Yunsong Wang (https://github.com/PointKernel) URL: #11482
davidwendt
approved these changes
Aug 5, 2022
… THRUST_DOUBLE_INDEX_TYPE_DISPATCH.
|
I have benchmarked commits 7c621d0 (Thrust 1.17, changes in this PR) and 217243c (Thrust 1.15, base). The results are below. Summary: I don't see anything too significant or worrisome resulting from the updated Thrust version. I ran with 1 iteration of the benchmarks (total runtime: 95 minutes for each commit on my machine). The results below show any changes that are outside the bounds of -7% to +5% runtime. A lot of these "changes" are statistical noise that happens more readily for smaller inputs, or I/O benchmarks that I do not expect to be stable on my system's SSD. Top Changes |
bdice
commented
Aug 9, 2022
|
rerun tests |
ttnghia
approved these changes
Aug 9, 2022
robertmaynard
approved these changes
Aug 9, 2022
|
@gpucibot merge |
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
Updates the bundled version of Thrust to 1.17.0. I will run benchmarks and include results in a comment below.
Depends on #11457.
Supersedes #10489, #10577, #10586. Closes #10841. This should be merged concurrently with rapidsai/rapids-cmake#231.
Checklist