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.

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

Use kvikIO as the default IO backend #12574

Merged
merged 8 commits into from
Jan 23, 2023

Conversation

vuule
Copy link
Contributor

@vuule vuule commented Jan 18, 2023

Description

Closes #10415
Switching the default backend to kvikIO.
Benchmarks show the same or better throughput.

Removing the redundant libcudf tests that ran with kvikIO.

Checklist

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

@vuule vuule added feature request New feature or request cuIO cuIO issue Performance Performance related issue labels Jan 18, 2023
@vuule vuule self-assigned this Jan 18, 2023
@vuule vuule added the non-breaking Non-breaking change label Jan 18, 2023
@github-actions github-actions bot added ci libcudf Affects libcudf (C++/CUDA) code. labels Jan 18, 2023
@codecov
Copy link

codecov bot commented Jan 19, 2023

Codecov Report

Base: 86.58% // Head: 85.71% // Decreases project coverage by -0.87% ⚠️

Coverage data is based on head (50bdf1f) compared to base (b6dccb3).
Patch has no changes to coverable lines.

Additional details and impacted files
@@               Coverage Diff                @@
##           branch-23.02   #12574      +/-   ##
================================================
- Coverage         86.58%   85.71%   -0.87%     
================================================
  Files               155      155              
  Lines             24368    24865     +497     
================================================
+ Hits              21098    21312     +214     
- Misses             3270     3553     +283     
Impacted Files Coverage Δ
python/cudf/cudf/_version.py 1.41% <0.00%> (-98.59%) ⬇️
python/cudf/cudf/core/buffer/spill_manager.py 72.50% <0.00%> (-7.50%) ⬇️
python/cudf/cudf/core/buffer/spillable_buffer.py 91.07% <0.00%> (-1.78%) ⬇️
python/cudf/cudf/utils/dtypes.py 77.85% <0.00%> (-1.61%) ⬇️
python/cudf/cudf/options.py 86.11% <0.00%> (-1.59%) ⬇️
python/cudf/cudf/core/single_column_frame.py 94.30% <0.00%> (-1.27%) ⬇️
...ython/custreamz/custreamz/tests/test_dataframes.py 98.38% <0.00%> (-1.01%) ⬇️
python/dask_cudf/dask_cudf/io/csv.py 96.34% <0.00%> (-1.00%) ⬇️
python/dask_cudf/dask_cudf/io/parquet.py 91.81% <0.00%> (-0.59%) ⬇️
python/cudf/cudf/core/multiindex.py 91.66% <0.00%> (-0.51%) ⬇️
... and 45 more

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

rapids-bot bot pushed a commit that referenced this pull request Jan 20, 2023
…ce_read()` (#12584)

`datasource_chunk_reader.get_next_chunk()` called `device_read_async()` without waiting on the returned Future.

Should fix the CI fail in #12574

cc. @vuule

Authors:
  - Mads R. B. Kristensen (https://github.com/madsbk)

Approvers:
  - AJ Schmidt (https://github.com/ajschmidt8)
  - Vukasin Milovanovic (https://github.com/vuule)
  - Yunsong Wang (https://github.com/PointKernel)

URL: #12584
@vuule vuule marked this pull request as ready for review January 20, 2023 23:41
@vuule vuule requested review from a team as code owners January 20, 2023 23:41
@vuule vuule requested review from vyasr and ttnghia January 20, 2023 23:41
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.

Changes look good.

I am not sure if this is the entire change required to change the default backend.
Changes in this PR looks good to me.

Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

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

quick comment about the CI script changes.

@@ -167,13 +167,6 @@ if [[ -z "$PROJECT_FLASH" || "$PROJECT_FLASH" == "0" ]]; then
echo "Running GoogleTest $test_name"
${gt} --gtest_output=xml:"$WORKSPACE/test-results/"
done

Copy link
Member

Choose a reason for hiding this comment

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

This file is no longer in use by CI since the switch to GitHub Actions.

the new CPP test file is here: https://github.com/rapidsai/cudf/blob/branch-23.02/ci/test_cpp.sh.

I imagine you may want to make similar changes there.

We will be removing all of the old Jenkins CI scripts from the RAPIDS repositories in the coming weeks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, done.

@vuule vuule requested a review from ajschmidt8 January 23, 2023 20:58
@vuule vuule added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Jan 23, 2023
@vuule
Copy link
Contributor Author

vuule commented Jan 23, 2023

/merge

@rapids-bot rapids-bot bot merged commit 7f6ae05 into rapidsai:branch-23.02 Jan 23, 2023
@vuule vuule deleted the fea-kvikio-default branch January 23, 2023 23:56
vuule added a commit that referenced this pull request Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge cuIO cuIO issue feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change Performance Performance related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] Use KvikIO by default
4 participants