Add a libcudf/thrust-based TPC-H derived datagen #16294

JayjeetAtGithub · 2024-07-16T21:54:25Z

Description

This PR adds a TPC-H (according to spec 3.0.1) inspired datagen written using libcudf and thrust

Implementation Status

Checklist

I am familiar with the Contributing Guidelines.
New or existing tests cover these changes.
The documentation is up to date with these changes.

copy-pr-bot · 2024-07-16T21:54:29Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

JayjeetAtGithub · 2024-07-24T22:25:10Z

TODO:

Complete the lineitem and orders tables
The stream and mr params need to be passed to the column generation functions throughout the code
Need to support scale factors <1

davidwendt

Could you move the function definitions/implementations from header files to .cpp/.cu files? It appears that dbgen.cu is a .cu file because of header it includes and that header contains device code which seems unnecessarily restrictive.

cpp/benchmarks/common/cudf_datagen/dbgen.cu

cpp/benchmarks/common/cudf_datagen/schema.hpp

cpp/benchmarks/common/tpch_data_generator/random_column_generator.cu

cpp/benchmarks/common/tpch_data_generator/random_column_generator.hpp

cpp/benchmarks/common/tpch_data_generator/tpch_datagen.hpp

cpp/benchmarks/common/tpch_data_generator/table_helpers.hpp

cpp/benchmarks/common/cudf_tpch_datagen/random_column_generator.hpp

…tor.hpp Co-authored-by: Karthikeyan <[email protected]>

harrism

This is a lot to get your head around and to build cleanly, so kudos to you @JayjeetAtGithub for getting it done!

Really I would have expected us to build this in Python, and I'm not sure why we didn't, but I suppose it's a good SOL test to do it in libcudf.

Overall it looks good, I had a couple of questions, but I approve.

Aside: I think that many of the generate_* function implementations have very high cognitive complexity so breaking them up into subfunctions might help maintainability (but I guess there's not much reuse potential). I don't think this is necessary before merging.

cpp/benchmarks/common/tpch_data_generator/random_column_generator.cu

cpp/benchmarks/common/tpch_data_generator/table_helpers.hpp

cpp/benchmarks/common/tpch_data_generator/tpch_data_generator.cpp

JayjeetAtGithub · 2024-08-29T18:04:29Z

Can we use std::span?

I tried using cudf::host_span, but looks like in the perform_left_join code, we use the column indices in the left_on and right_on params to select columns from a table using table_view.select. Now table_view.select accepts only a vector. So we can't use std::span here.

Addressed

PointKernel

Thanks for the great work. This is a big step forward, and I'm really excited about the upcoming NVBench migration.

@JayjeetAtGithub, could you please document the concerns we discussed in a GitHub issue to ensure we won't miss them in the future?

cpp/benchmarks/common/tpch_data_generator/tpch_data_generator.cpp

karthikeyann · 2024-08-29T22:40:22Z

cpp/benchmarks/common/tpch_data_generator/tpch_data_generator.cpp

+    auto const gather_map     = cudf::table_view({indices->view(), keys->view()});
+    auto const gathered_table = cudf::gather(
+      gather_map, ternary_mask->view(), cudf::out_of_bounds_policy::DONT_CHECK, stream, mr);
+    auto gathered_table_columns = gathered_table->release();
+    return std::move(gathered_table_columns[1]);


nit: indices is not required here in gather_map table.

karthikeyann · 2024-08-29T22:40:40Z

cpp/benchmarks/common/tpch_data_generator/tpch_data_generator.cpp

+    auto const gather_map     = cudf::table_view({indices->view(), keys->view()});
+    auto const gathered_table = cudf::gather(
+      gather_map, mask_index_type->view(), cudf::out_of_bounds_policy::DONT_CHECK, stream, mr);
+    auto gathered_table_columns = gathered_table->release();


nit: indices is not required here in gather_map table.

cpp/benchmarks/common/tpch_data_generator/tpch_data_generator.cpp

vyasr

Reviewing just the CMake.

How do you anticipate the generator be used? Will it just be used by other benchmarks in our repository? If so, the CMake looks fine as-is. If you intend broader usage (I'd caution against that unless we have a very good reason) then there is some additional machinery.

JayjeetAtGithub · 2024-08-29T23:17:44Z

Reviewing just the CMake.

How do you anticipate the generator be used? Will it just be used by other benchmarks in our repository? If so, the CMake looks fine as-is. If you intend broader usage (I'd caution against that unless we have a very good reason) then there is some additional machinery.

Thanks for taking a look @vyasr. For now, we only want our internal benchmark code to use the datagen by just linking to the static library libtpch_data_generator.a.

karthikeyann

Looks good to me 👍
Nice work! This is going to be very useful for benchmarks.

karthikeyann · 2024-08-30T02:27:01Z

/merge

github-actions bot added libcudf Affects libcudf (C++/CUDA) code. CMake CMake build issue labels Jul 16, 2024

JayjeetAtGithub added feature request New feature or request non-breaking Non-breaking change labels Jul 16, 2024

JayjeetAtGithub changed the title ~~Add a libcudf/thrust-based TPC-H inspired datagen~~ [WIP] Add a libcudf/thrust-based TPC-H inspired datagen Jul 16, 2024

GregoryKimball requested a review from karthikeyann July 17, 2024 19:38

JayjeetAtGithub force-pushed the cudf_datagen branch from a834875 to e45c419 Compare July 22, 2024 21:51

JayjeetAtGithub marked this pull request as ready for review July 24, 2024 22:24

JayjeetAtGithub requested a review from a team as a code owner July 24, 2024 22:24

JayjeetAtGithub requested a review from ttnghia July 24, 2024 22:24

JayjeetAtGithub changed the base branch from branch-24.08 to branch-24.10 July 24, 2024 23:48

JayjeetAtGithub changed the title ~~[WIP] Add a libcudf/thrust-based TPC-H inspired datagen~~ Add a libcudf/thrust-based TPC-H inspired datagen Jul 27, 2024

JayjeetAtGithub force-pushed the cudf_datagen branch from 1e7abc5 to cd5ec3c Compare August 1, 2024 00:12

davidwendt previously requested changes Aug 6, 2024

View reviewed changes

ttnghia reviewed Aug 8, 2024

View reviewed changes

cpp/benchmarks/common/cudf_datagen/dbgen.cu Outdated Show resolved Hide resolved

ttnghia reviewed Aug 8, 2024

View reviewed changes

cpp/benchmarks/common/cudf_datagen/schema.hpp Outdated Show resolved Hide resolved

JayjeetAtGithub changed the title ~~Add a libcudf/thrust-based TPC-H inspired datagen~~ Add a libcudf/thrust-based TPC-H deriveddatagen Aug 8, 2024

JayjeetAtGithub changed the title ~~Add a libcudf/thrust-based TPC-H deriveddatagen~~ Add a libcudf/thrust-based TPC-H derived datagen Aug 8, 2024

JayjeetAtGithub requested a review from a team as a code owner August 8, 2024 19:46

JayjeetAtGithub and others added 9 commits August 8, 2024 23:14

Generate part, partsupp, customer, supplier, nation, region tables

cbf25da

Add consts

455df49

Add consts

88ecc8a

Fix consts placements

d779512

Add note about incomplete table

bdbe0e9

Can generate the timestamps cols

d2dd908

Generate a mask based on predicate

b2d6b56

Generate l_linestatus col

963a160

Generate l_returnflag col

fd761cc

JayjeetAtGithub requested a review from karthikeyann August 28, 2024 16:37

Rename to tpch_data_generator

1c408a5

karthikeyann reviewed Aug 28, 2024

View reviewed changes

JayjeetAtGithub and others added 4 commits August 28, 2024 11:12

Use nested namespaces

7bf394d

Update cpp/benchmarks/common/tpch_data_generator/random_column_genera…

736ddb9

…tor.hpp Co-authored-by: Karthikeyan <[email protected]>

Rename files

0252522

Use release and move

6ba23f3

JayjeetAtGithub requested a review from karthikeyann August 28, 2024 18:50

harrism approved these changes Aug 28, 2024

View reviewed changes

JayjeetAtGithub added 2 commits August 29, 2024 09:41

Undo the structured binding

1cee77e

Add back indices

99cb18c

Use macro and remove __host__ where not needed

b57dfda

JayjeetAtGithub and others added 2 commits August 29, 2024 12:15

Remove datagen.cpp

38b6372

Merge branch 'branch-24.10' into cudf_datagen

3e83f87

PointKernel approved these changes Aug 29, 2024

View reviewed changes

JayjeetAtGithub requested review from vyasr, bdice and KyleFromNVIDIA August 29, 2024 22:33

karthikeyann reviewed Aug 29, 2024

View reviewed changes

Remove couts

6e0df3c

vyasr reviewed Aug 29, 2024

View reviewed changes

remove usless code

91c5bd3

JayjeetAtGithub requested a review from vyasr August 29, 2024 23:18

karthikeyann approved these changes Aug 30, 2024

View reviewed changes

rapids-bot bot merged commit 23fb31e into rapidsai:branch-24.10 Aug 30, 2024
86 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a libcudf/thrust-based TPC-H derived datagen #16294

Add a libcudf/thrust-based TPC-H derived datagen #16294

JayjeetAtGithub commented Jul 16, 2024 •

edited

Loading

copy-pr-bot bot commented Jul 16, 2024

JayjeetAtGithub commented Jul 24, 2024 •

edited

Loading

davidwendt left a comment

harrism left a comment •

edited

Loading

JayjeetAtGithub commented Aug 29, 2024

PointKernel left a comment

karthikeyann Aug 29, 2024 •

edited

Loading

karthikeyann Aug 29, 2024 •

edited

Loading

vyasr left a comment

JayjeetAtGithub commented Aug 29, 2024

karthikeyann left a comment

karthikeyann commented Aug 30, 2024

Add a libcudf/thrust-based TPC-H derived datagen #16294

Add a libcudf/thrust-based TPC-H derived datagen #16294

Conversation

JayjeetAtGithub commented Jul 16, 2024 • edited Loading

Description

Implementation Status

Checklist

copy-pr-bot bot commented Jul 16, 2024

JayjeetAtGithub commented Jul 24, 2024 • edited Loading

davidwendt left a comment

Choose a reason for hiding this comment

harrism left a comment • edited Loading

Choose a reason for hiding this comment

JayjeetAtGithub commented Aug 29, 2024

PointKernel left a comment

Choose a reason for hiding this comment

karthikeyann Aug 29, 2024 • edited Loading

Choose a reason for hiding this comment

karthikeyann Aug 29, 2024 • edited Loading

Choose a reason for hiding this comment

vyasr left a comment

Choose a reason for hiding this comment

JayjeetAtGithub commented Aug 29, 2024

karthikeyann left a comment

Choose a reason for hiding this comment

karthikeyann commented Aug 30, 2024

JayjeetAtGithub commented Jul 16, 2024 •

edited

Loading

JayjeetAtGithub commented Jul 24, 2024 •

edited

Loading

harrism left a comment •

edited

Loading

karthikeyann Aug 29, 2024 •

edited

Loading

karthikeyann Aug 29, 2024 •

edited

Loading