Add device create_sequence_table for benchmarks #10300
Conversation
karthikeyann
added
feature request
Codecov Report
@@ Coverage Diff @@
## branch-22.04 #10300 +/- ##
================================================
- Coverage 10.62% 10.62% -0.01%
================================================
Files 122 122
Lines 20961 20973 +12
================================================
Hits 2228 2228
- Misses 18733 18745 +12
Continue to review full report at Codecov.
|
|
rerun tests |
seperate nullmask generator code This reverts commit 916ce00.
| #include <thrust/random.h> | ||
|
|
||
| /** | ||
| * @brief valid bit generator with given probability [0.0 - 1.0] |
There was a problem hiding this comment.
I'd change the docs / naming here. There's no such thing as a "valid" bit -- only set (1) or unset (0) bits. This is layered with our interpretation of a set of bits as a null mask, but this function as written only deals with generating bools. (Moreover, this generates a byte-size bool and not a "bit".)
This mixing of semantics was a large part of what I untangled in #9588. In particular, there were issues with how that interpretation of valid/set and null/unset results in higher-order semantics when a null mask is nullptr. It's not possible to count "set" bits in the data pointed to by nullptr, but we have a specific interpretation of that in the context of validity / cudf null masks.
I want to emphasize this distinction and keep our lowest-level primitives (such as random generators) free of the valid/null interpretations of bits/bools. Including those semantics in functions like create_random_null_mask is fine, because the interpretation is explicitly intended and communicated in the function name / docstring.
|
all review comments addressed. |
|
@gpucibot merge |
Fixes `BINARYOP_BENCH` which is throwing an error for non-numeric types: ``` terminate called after throwing an instance of 'cudf::logic_error' what(): cuDF failure at: /cudf/cpp/src/filling/sequence.cu:139: init scalar type must be numeric ``` The `compiled_binaryop.cpp` was recently changed in #10300 to create test columns using the benchmark utility `create_sequence_table` which internally calls `cudf::sequence` API. Unfortunately, [only `numeric` types can be used with this API](https://github.com/rapidsai/cudf/blob/a9b6cb113bcacecd0752d2957971c0d417cf719e/cpp/src/filling/sequence.cu#L139) which throws an error for types like `timestamp, duration, and decimal` which are being measured in this file. https://docs.rapids.ai/api/libcudf/stable/group__transformation__fill.html#gaeda630c9dcdd152eeecf0a1b636244ac The fix replaces the `create_sequence_table` call with `create_random_table` to generate the source columns instead. Authors: - David Wendt (https://github.com/davidwendt) Approvers: - Karthikeyan (https://github.com/karthikeyann) - Nghia Truong (https://github.com/ttnghia) URL: #10398
To speedup generate benchmark input generation, move all data generation to device. To address #5773 (comment) This PR moves the random input generation to device. Rest all of the original work in this PR was split to multiple PRs and merged. #10277 #10278 #10279 #10280 #10281 #10300 With all of these changes, single iteration of all benchmark runs in <1000 seconds. (from 3067s to 964s). Running more iterations would see higher benefit too because the benchmark is restarted several times during run which again calls benchmark input generation code. closes #9857 Authors: - Karthikeyan (https://github.com/karthikeyann) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) - Vukasin Milovanovic (https://github.com/vuule) - David Wendt (https://github.com/davidwendt) URL: #10109
addresses parts of #5773
create_sequence_tablewhich creates sequences in device (only numeric types supported) with/without nulls.create_random_null_maskto create random null mask with given probability. (0.0-1.0 null probability)- add gnu++17 to generate_input.cu (temporarily for int128 STL support).repeat_dtypestocycle_dtypesand moved out of create_* methodsSplitting PR #10109 for review