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

Fix contiguous_split not properly handling output partitions > 2 GB. #7515

Merged
merged 2 commits into from
Mar 10, 2021
Merged

Fix contiguous_split not properly handling output partitions > 2 GB. #7515

merged 2 commits into from
Mar 10, 2021

Conversation

nvdbaranec
Copy link
Contributor

Fixes:
#7514

Related:
NVIDIA/spark-rapids#1861

There were a couple of places where 32 bit values were being used for buffer sizes that needed to be 64 bit.

@nvdbaranec nvdbaranec added bug Something isn't working 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change labels Mar 4, 2021
@nvdbaranec nvdbaranec requested a review from a team as a code owner March 4, 2021 20:33
@nvdbaranec nvdbaranec requested review from jrhemstad and vuule March 4, 2021 20:33
@nvdbaranec nvdbaranec requested review from jrhemstad and vuule March 4, 2021 22:19
vuule
vuule approved these changes Mar 4, 2021
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.

I wish we used auto more - this PR would have been much smaller.

@nvdbaranec
Copy link
Contributor Author

rerun tests

2 similar comments
@nvdbaranec
Copy link
Contributor Author

rerun tests

@nvdbaranec
Copy link
Contributor Author

rerun tests

@codecov
Copy link

codecov bot commented Mar 9, 2021 •
edited
Loading

Codecov Report

Merging #7515 (b505147) into branch-0.19 (43b44e1) will increase coverage by 0.46%.
The diff coverage is 87.98%.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.19    #7515      +/-   ##
===============================================
+ Coverage        81.80%   82.27%   +0.46%     
===============================================
  Files              101      101              
  Lines            16695    17261     +566     
===============================================
+ Hits             13658    14202     +544     
- Misses            3037     3059      +22
Impacted Files Coverage Δ
python/cudf/cudf/core/column/string.py 86.76% <ø> (+0.39%) ⬆️
python/cudf/cudf/utils/docutils.py 97.36% <50.00%> (-2.64%) ⬇️
python/cudf/cudf/testing/testing.py 80.00% <57.14%> (-1.04%) ⬇️
python/cudf/cudf/core/column/timedelta.py 88.57% <75.00%> (+0.33%) ⬆️
python/cudf/cudf/core/column/categorical.py 91.74% <77.14%> (-0.49%) ⬇️
python/cudf/cudf/core/column/datetime.py 89.63% <78.57%> (+0.58%) ⬆️
python/dask_cudf/dask_cudf/core.py 71.67% <79.41%> (-2.60%) ⬇️
python/cudf/cudf/core/multiindex.py 82.90% <90.90%> (+0.73%) ⬆️
python/cudf/cudf/core/tools/datetimes.py 84.53% <93.33%> (+0.89%) ⬆️
python/cudf/cudf/core/column_accessor.py 95.47% <95.65%> (+2.53%) ⬆️
... and 67 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c69b6f8...b505147. Read the comment docs.

@ttnghia
Copy link
Contributor

ttnghia commented Mar 9, 2021

Rerun tests.

@nvdbaranec nvdbaranec added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Mar 10, 2021
@nvdbaranec
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 42c6d15 into rapidsai:branch-0.19 Mar 10, 2021
@sameerz sameerz mentioned this pull request Mar 11, 2021
Closed
hyperbolic2346 pushed a commit to hyperbolic2346/cudf that referenced this pull request Mar 25, 2021
@nvdbaranec @hyperbolic2346
Fix contiguous_split not properly handling output partitions > 2 GB. (r…
34f6de8 
…apidsai#7515)

Fixes:
rapidsai#7514

Related:
NVIDIA/spark-rapids#1861

There were a couple of places where 32 bit values were being used for buffer sizes that needed to be 64 bit.

Authors:
  - @nvdbaranec

Approvers:
  - Vukasin Milovanovic (@vuule)
  - Jake Hemstad (@jrhemstad)

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

@vuule vuule vuule approved these changes

@jrhemstad jrhemstad jrhemstad approved these changes

Assignees
No one assigned
Labels
3 - Ready for Review Ready for review by team 5 - Ready to Merge Testing and reviews complete, ready to merge bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nvdbaranec @ttnghia @jrhemstad @vuule