Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Convert compression and io to string axis type in IO benchmarks #14347

Merged
merged 13 commits into from
Dec 11, 2023
Merged

Convert compression and io to string axis type in IO benchmarks #14347

merged 13 commits into from
Dec 11, 2023

Conversation

SurajAralihalli
Copy link
Contributor

@SurajAralihalli SurajAralihalli commented Oct 30, 2023 •
edited
Loading

Addresses issue: #12739

This PR transforms compression and io into string axis types to enable the selection of different values via the CLI, eliminating the need to execute all values in an automation when required. Additionally, this PR introduces two new functions, retrieve_io_type_enum and retrieve_compression_type_enum, which facilitate the conversion of string input into the corresponding enum type that can be used in benchmarking functions.

IO Benchmarks:

  • PARQUET READER

For example:
./PARQUET_READER_NVBENCH -b parquet_read_io_compression --axis io_type=[HOST_BUFFER] --axis compression_type=[NONE]

Checklist

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

@SurajAralihalli
use string axis for io type in BM_parquet_read_data
0ec95d6 
Signed-off-by: Suraj Aralihalli <suraj.ara16@gmail.com>
@copy-pr-bot
Copy link

copy-pr-bot bot commented Oct 30, 2023

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.

@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Oct 30, 2023
@SurajAralihalli
update header file
b9f911d 
Signed-off-by: Suraj Aralihalli <suraj.ara16@gmail.com>
@davidwendt davidwendt added 2 - In Progress Currently a work in progress improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Oct 30, 2023
@davidwendt
Copy link
Contributor

/ok to test

@SurajAralihalli
make BM_parquet_read_data working
9206a34 
Signed-off-by: Suraj Aralihalli <suraj.ara16@gmail.com>
@SurajAralihalli
add string axis type to all benchmarks in parquet_reader_input.cpp
ec0fe55 
Signed-off-by: Suraj Aralihalli <suraj.ara16@gmail.com>
@SurajAralihalli
Copy link
Contributor Author

SurajAralihalli commented Oct 31, 2023 •
edited
Loading

This modification allows for the control of the io and compression types in PARQUET_READER_NVBENCH benchmarks using CLI. For instance:

./PARQUET_READER_NVBENCH -b parquet_read_io_compression --axis io_type=[HOST_BUFFER] --axis compression_type=[NONE]

@SurajAralihalli
Copy link
Contributor Author

@GregoryKimball Can you review the PR please, I can update other benchmark types, if this looks right to you!

@GregoryKimball
Copy link
Contributor

Thank you @SurajAralihalli for this change. I recommend just starting with the parquet reader benchmarks to begin with. @vuule and I were discussing adding a V1/V2 header parameter as well, but that can be in a follow-on PR.

@vuule vuule self-requested a review November 3, 2023 20:57
vuule
vuule approved these changes Nov 8, 2023
View reviewed changes
Copy link
Contributor

@vuule vuule left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you!

@SurajAralihalli
cuda fail when incorrect io/compression type is used
6783527 
Signed-off-by: Suraj Aralihalli <suraj.ara16@gmail.com>
@SurajAralihalli
Copy link
Contributor Author

Sorry for my delayed response.

@vuule If this pull request aligns with the existing requirements please feel free to run the tests and merge the pull request.

@SurajAralihalli SurajAralihalli marked this pull request as ready for review November 14, 2023 23:00
@SurajAralihalli SurajAralihalli requested a review from a team as a code owner November 14, 2023 23:00
@vuule
Copy link
Contributor

vuule commented Nov 15, 2023

/ok to test

@vuule vuule added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Nov 15, 2023
@vuule vuule changed the base branch from branch-23.12 to branch-24.02 November 15, 2023 17:45
@vuule vuule requested review from a team as code owners November 15, 2023 17:45
@github-actions github-actions bot removed the ci label Nov 15, 2023
@SurajAralihalli SurajAralihalli marked this pull request as ready for review November 15, 2023 23:46
karthikeyann
karthikeyann reviewed Nov 17, 2023
View reviewed changes
Copy link
Contributor

@karthikeyann karthikeyann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should reduce the compile time slightly too since the template arguments are reduced.

cpp/benchmarks/io/cuio_common.cpp Outdated Show resolved Hide resolved
@wence- wence- removed request for a team, vyasr and charlesbluca November 17, 2023 13:00
@karthikeyann
Copy link
Contributor

/ok to test

@karthikeyann
Copy link
Contributor

/ok to test

@SurajAralihalli
static cast int64 to int32
c4ab7a7 
Signed-off-by: Suraj Aralihalli <suraj.ara16@gmail.com>
@SurajAralihalli
use auto
585c328 
Signed-off-by: Suraj Aralihalli <suraj.ara16@gmail.com>
@vuule
Copy link
Contributor

vuule commented Dec 8, 2023

/ok to test

@vuule
Copy link
Contributor

vuule commented Dec 11, 2023

/merge

@rapids-bot rapids-bot bot merged commit 3c32e5d into rapidsai:branch-24.02 Dec 11, 2023
67 checks passed
karthikeyann pushed a commit to karthikeyann/cudf that referenced this pull request Dec 12, 2023
@SurajAralihalli @karthikeyann
Convert compression and io to string axis type in IO benchmarks (rapi…
3a2404d 
…dsai#14347)

Addresses issue: [rapidsai#12739](rapidsai#12739)


This PR transforms compression and io into string axis types to enable the selection of different values via the CLI, eliminating the need to execute all values in an automation when required.  Additionally, this PR introduces two new functions, `retrieve_io_type_enum` and `retrieve_compression_type_enum`, which facilitate the conversion of string input into the corresponding enum type that can be used in benchmarking functions.

IO Benchmarks:
- [x] PARQUET READER 


For example:
`./PARQUET_READER_NVBENCH -b parquet_read_io_compression --axis io_type=[HOST_BUFFER] --axis compression_type=[NONE]`

Authors:
  - Suraj Aralihalli (https://github.com/SurajAralihalli)

Approvers:
  - Vukasin Milovanovic (https://github.com/vuule)
  - Nghia Truong (https://github.com/ttnghia)

URL: rapidsai#14347
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karthikeyann karthikeyann karthikeyann left review comments

@ttnghia ttnghia ttnghia approved these changes

@vuule vuule vuule approved these changes

@divyegala divyegala Awaiting requested review from divyegala divyegala is a code owner automatically assigned from rapidsai/cudf-cpp-codeowners

Assignees
No one assigned
Labels
3 - Ready for Review Ready for review by team improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
cuDF/Dask/Numba/UCX
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@SurajAralihalli @davidwendt @GregoryKimball @vuule @karthikeyann @ttnghia