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

bitmask_or implementation with bitmask refactor #7406

Merged
merged 10 commits into from
Mar 8, 2021

Conversation

rwlee
Copy link
Contributor

@rwlee rwlee commented Feb 18, 2021

Refactors the bitmask merging functionality to support any binary function, allowing for bitwise_or support in addition the existing bitwise_and support. Includes changes to the Java api and JNI to access the bitwise_or functionality.

@rwlee rwlee added 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. Java Affects Java cuDF API. Spark Functionality that helps Spark RAPIDS improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Feb 18, 2021
@rwlee rwlee requested review from a team as code owners February 18, 2021 01:45
@rwlee rwlee requested review from cwharris and vuule February 18, 2021 01:46
@rwlee rwlee force-pushed the rwlee/cleanvalidity19 branch from cd9210c to d3382e0 Compare February 18, 2021 04:15
@codecov
Copy link

codecov bot commented Feb 18, 2021

Codecov Report

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

Impacted file tree graph

@@              Coverage Diff               @@
##             branch-0.19    #7406   +/-   ##
==============================================
  Coverage               ?   81.86%           
==============================================
  Files                  ?      101           
  Lines                  ?    16884           
  Branches               ?        0           
==============================================
  Hits                   ?    13822           
  Misses                 ?     3062           
  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 4cd5f8d...b697b0e. Read the comment docs.

cpp/include/cudf/detail/null_mask.hpp Outdated Show resolved Hide resolved
cpp/src/bitmask/null_mask.cu Outdated Show resolved Hide resolved
Copy link
Member

@jlowe jlowe left a comment

Choose a reason for hiding this comment

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

Java approval

@jlowe jlowe changed the title [REVIEW] bitmask_or implementation with bitmask refactor bitmask_or implementation with bitmask refactor Mar 2, 2021
Copy link
Contributor

@jrhemstad jrhemstad left a comment

Choose a reason for hiding this comment

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

Just move the sync and this looks good to go. Thanks for doing the extra work of cleaning up with the device_span/host_span.

@rwlee
Copy link
Contributor Author

rwlee commented Mar 3, 2021

rerun tests -- cuda 11.0 python 3.8 Ubuntu 18.04 errored with Error response from daemon: Get https://registry-1.docker.io/v2/gpuci/rapidsai/manifests/0.19-cuda11.0-devel-ubuntu18.04-py3.8: unauthorized: incorrect username or password

@rwlee
Copy link
Contributor Author

rwlee commented Mar 3, 2021

Rerun tests

@rwlee
Copy link
Contributor Author

rwlee commented Mar 4, 2021

rerun tests

@rwlee rwlee added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Mar 4, 2021
@rwlee
Copy link
Contributor Author

rwlee commented Mar 8, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 3480e2e into rapidsai:branch-0.19 Mar 8, 2021
rapids-bot bot pushed a commit that referenced this pull request Mar 9, 2021
Fix for issue caused by stale PR issue from #7406

Authors:
  - @rwlee
  - Keith Kraus (@kkraus14)

Approvers:
  - Keith Kraus (@kkraus14)
  - Mike Wilson (@hyperbolic2346)
  - GALI PREM SAGAR (@galipremsagar)
  - Jake Hemstad (@jrhemstad)
  - Vukasin Milovanovic (@vuule)
  - Paul Taylor (@trxcllnt)

URL: #7533
hyperbolic2346 pushed a commit to hyperbolic2346/cudf that referenced this pull request Mar 25, 2021
Refactors the bitmask merging functionality to support any binary function, allowing for `bitwise_or` support in addition the existing `bitwise_and` support. Includes changes to the Java api and JNI to access the `bitwise_or` functionality.

Authors:
  - @rwlee

Approvers:
  - Jason Lowe (@jlowe)
  - Jake Hemstad (@jrhemstad)
  - Christopher Harris (@cwharris)

URL: rapidsai#7406
hyperbolic2346 pushed a commit to hyperbolic2346/cudf that referenced this pull request Mar 25, 2021
Fix for issue caused by stale PR issue from rapidsai#7406

Authors:
  - @rwlee
  - Keith Kraus (@kkraus14)

Approvers:
  - Keith Kraus (@kkraus14)
  - Mike Wilson (@hyperbolic2346)
  - GALI PREM SAGAR (@galipremsagar)
  - Jake Hemstad (@jrhemstad)
  - Vukasin Milovanovic (@vuule)
  - Paul Taylor (@trxcllnt)

URL: rapidsai#7533
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 improvement Improvement / enhancement to an existing function Java Affects Java cuDF API. libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change Spark Functionality that helps Spark RAPIDS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants