Skip to content

Fix struct binary search and struct flattening - #8268

Merged
rapids-bot[bot] merged 13 commits into
NVIDIA:branch-21.06from
ttnghia:fix_struct_comparison
May 24, 2021
Merged

Fix struct binary search and struct flattening#8268
rapids-bot[bot] merged 13 commits into
NVIDIA:branch-21.06from
ttnghia:fix_struct_comparison

Conversation

@ttnghia

@ttnghia ttnghia commented May 18, 2021

Copy link
Copy Markdown
Contributor

This PR fixes several bugs. In particular:

  • Fixes a bug in struct binary search that only check for null elements at the top level.
  • Fixes a bug in struct flattening that uses default null order for the children of the input structs columns.

Unit tests for struct binary search and struct sorting are also rewritten.
Closes #8189.

@ttnghia ttnghia added bug Something isn't working 3 - Ready for Review Ready for review by team tests Unit testing for project libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change labels May 18, 2021
@ttnghia ttnghia self-assigned this May 18, 2021
@ttnghia
ttnghia requested a review from a team as a code owner May 18, 2021 03:51
@ttnghia
ttnghia requested review from harrism and jrhemstad May 18, 2021 03:51
@ttnghia ttnghia changed the title Fix struct comparison Fix struct binary search and struct flattening May 18, 2021
Comment thread cpp/src/search/search.cu Outdated
@ttnghia
ttnghia requested a review from harrism May 18, 2021 14:32
Comment thread cpp/include/cudf/column/column.hpp Outdated
@codecov

codecov Bot commented May 18, 2021

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (branch-21.06@ef20706). Click here to learn what that means.
The diff coverage is n/a.

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

@@               Coverage Diff               @@
##             branch-21.06    #8268   +/-   ##
===============================================
  Coverage                ?   82.89%           
===============================================
  Files                   ?      105           
  Lines                   ?    17875           
  Branches                ?        0           
===============================================
  Hits                    ?    14818           
  Misses                  ?     3057           
  Partials                ?        0           

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 ef20706...000f62b. Read the comment docs.

@revans2 revans2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

From my perspective things look good. I have rerun tests and they pass.

@harrism

harrism commented May 24, 2021

Copy link
Copy Markdown
Contributor

@gpucibot merge

@rapids-bot
rapids-bot Bot merged commit dd5eecd into NVIDIA:branch-21.06 May 24, 2021
@ttnghia
ttnghia deleted the fix_struct_comparison branch May 27, 2021 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - Ready for Review Ready for review by team bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change tests Unit testing for project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] null ordering for nulls in children appears to be ignored

4 participants