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

Fix test failure with cuda 11.5 in row_bit_count tests. #9581

Merged

Conversation

nvdbaranec
Copy link
Contributor

@nvdbaranec nvdbaranec commented Nov 2, 2021

Test code was relying on a quirky (and probably incorrect) compiler behavior where

using LCW   = cudf::test::lists_column_wrapper<T, int>;
lists_column_wrapper<int> col{ LCW{LCW{}} };

would produce a List<List<int>> column with no rows at the top. With cuda 11.5 the order of constructors causes this to create a List<List<int>> column with 1 row at the top. This leaves lists_column_wrapper with no way to currently express "an entirely empty list column nested more than 1 level deep" so the fix for now is just to construct these tests manually.

…. Was relying on a quirky compiler behavior

with initializer list constructors that has been removed in cuda 11.5
@nvdbaranec nvdbaranec requested a review from a team as a code owner November 2, 2021 17:40
@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Nov 2, 2021
@nvdbaranec nvdbaranec added bug Something isn't working non-breaking Non-breaking change labels Nov 2, 2021
Copy link
Contributor

@codereport codereport left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@ttnghia
Copy link
Contributor

ttnghia commented Nov 2, 2021

Test code was relying on a quirky (and probably incorrect) compiler behavior where

using LCW   = cudf::test::lists_column_wrapper<T, int>;
lists_column_wrapper<int> col{ LCW{LCW{}} };

would produce a List<List<int>> column with no rows at the top. With cuda 11.5 the order of constructors causes this to create a List<List<int>> column with 1 row at the top. This leaves lists_column_wrapper with no way to currently express "an entirely empty list column nested more than 1 level deep" so the fix for now is just to construct these tests manually.

From my experience I had no way to create List<List<int>> with 0 row. col{ LCW{LCW{}} } always create one row containing an empty list.

@codecov
Copy link

codecov bot commented Nov 2, 2021

Codecov Report

Merging #9581 (82cc520) into branch-21.12 (ab4bfaa) will decrease coverage by 0.13%.
The diff coverage is n/a.

❗ Current head 82cc520 differs from pull request most recent head a5d4ea7. Consider uploading reports for the commit a5d4ea7 to get more accurate results
Impacted file tree graph

@@               Coverage Diff                @@
##           branch-21.12    #9581      +/-   ##
================================================
- Coverage         10.79%   10.65%   -0.14%     
================================================
  Files               116      117       +1     
  Lines             18869    19743     +874     
================================================
+ Hits               2036     2104      +68     
- Misses            16833    17639     +806     
Impacted Files Coverage Δ
python/dask_cudf/dask_cudf/sorting.py 92.90% <0.00%> (-1.21%) ⬇️
python/cudf/cudf/io/csv.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/hdf.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/orc.py 0.00% <0.00%> (ø)
python/cudf/cudf/__init__.py 0.00% <0.00%> (ø)
python/cudf/cudf/_version.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/abc.py 0.00% <0.00%> (ø)
python/cudf/cudf/api/types.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/dlpack.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/frame.py 0.00% <0.00%> (ø)
... and 68 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 e632f97...a5d4ea7. Read the comment docs.

@JohnZed
Copy link
Contributor

JohnZed commented Nov 2, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit cfcf90f into rapidsai:branch-21.12 Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants