Fix quantile gtests coded in namespace cudf::test #12049
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
davidwendt
added
2 - In Progress
3 tasks
Codecov ReportBase: 88.02% // Head: 88.04% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## branch-22.12 #12049 +/- ##
================================================
+ Coverage 88.02% 88.04% +0.02%
================================================
Files 135 135
Lines 22025 22025
================================================
+ Hits 19388 19393 +5
+ Misses 2637 2632 -5
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 at Codecov. |
davidwendt
added
3 - Ready for Review
bdice
approved these changes
Nov 7, 2022
ttnghia
approved these changes
Nov 7, 2022
|
@gpucibot merge |
vyasr
approved these changes
Nov 7, 2022
rapids-bot bot
pushed a commit
that referenced
this pull request
Nov 9, 2022
Changes `cudf::detail::tdigest` to `cudf::tdigest::detail` in the tdigest source files. While working on #12049, found there was a mixture of `cudf::tdigest` and `cudf::detail::tdigest` that seemed confusing and inconsistent. Changing to `cudf::tdigest::detail` made this code easier to follow. Also, move the `size_begin()` member function in `tdigest_column_view` out as a standalone function in a separate `.cuh` header since it is only used in a few places and the `tdigest_column_view.cuh` is included in many places. This allowed changing the `tdigest_column_view.cuh` to a `.hpp` file. Depends on #12049 Authors: - David Wendt (https://github.com/davidwendt) Approvers: - Bradley Dice (https://github.com/bdice) - Nghia Truong (https://github.com/ttnghia) - Ray Douglass (https://github.com/raydouglass) URL: #12050
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
Fixes
cpp/tests/quantilesgtests source files coded in namespacecudf::testThe
tdigest_utilities.cuwas moved tocpp/tests/utilitiessince it is used by quantiles, groupby, reductions tests. Also, the header for the functions defined in this source file is incpp/include/cudf_tests/.The
cpp/include/cudf_tests/tdigest_utilities.cuhwas also including a source file header fromcudf/tests/groupbywhich seemed odd and was corrected by moving the code it needed directly into thetdigest_utilities.cuhheader. These functions were used by quantiles, groupby, reductions, etc so it made sense for them to be moved into this utility header.Simple reworking some of the code in
percentile_approx_test.cuallowed it to become a.cppfile as well.Also made some minor changes to the
tdigest_column_viewclass to isolate a functor inside the class instead of the namespace scope.No function or test has changed just the source code reworked or moved around.
Reference #11734
Checklist