Skip to content

bitmask_or implementation with bitmask refactor - #7406

Merged
rapids-bot[bot] merged 10 commits into
NVIDIA:branch-0.19from
rwlee:rwlee/cleanvalidity19
Mar 8, 2021
Merged

bitmask_or implementation with bitmask refactor#7406
rapids-bot[bot] merged 10 commits into
NVIDIA:branch-0.19from
rwlee:rwlee/cleanvalidity19

Conversation

@rwlee

@rwlee rwlee commented Feb 18, 2021

Copy link
Copy Markdown
Contributor

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

codecov Bot commented Feb 18, 2021 •
edited
Loading

Copy link
Copy Markdown

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.

jrhemstad
jrhemstad requested changes Feb 18, 2021
View reviewed changes
Comment thread cpp/include/cudf/detail/null_mask.hpp Outdated
Comment thread cpp/src/bitmask/null_mask.cu Outdated
jrhemstad
jrhemstad reviewed Feb 23, 2021
View reviewed changes
Comment thread cpp/include/cudf/detail/null_mask.cuh Outdated
jrhemstad
jrhemstad reviewed Feb 23, 2021
View reviewed changes
Comment thread cpp/include/cudf/detail/null_mask.cuh Outdated
jrhemstad
jrhemstad reviewed Feb 23, 2021
View reviewed changes
Comment thread cpp/include/cudf/detail/null_mask.cuh Outdated
jrhemstad
jrhemstad reviewed Feb 23, 2021
View reviewed changes
Comment thread cpp/include/cudf/detail/null_mask.cuh Outdated
jrhemstad
jrhemstad reviewed Feb 23, 2021
View reviewed changes
Comment thread cpp/include/cudf/detail/null_mask.cuh Outdated
jrhemstad
jrhemstad reviewed Feb 23, 2021
View reviewed changes
Comment thread cpp/include/cudf/detail/null_mask.cuh
jrhemstad
jrhemstad reviewed Feb 23, 2021
View reviewed changes
Comment thread cpp/include/cudf/detail/null_mask.cuh
jrhemstad
jrhemstad reviewed Feb 23, 2021
View reviewed changes
Comment thread cpp/src/bitmask/null_mask.cu Outdated
jrhemstad
jrhemstad reviewed Feb 23, 2021
View reviewed changes
Comment thread cpp/src/bitmask/null_mask.cu Outdated
jrhemstad
jrhemstad reviewed Feb 23, 2021
View reviewed changes
Comment thread cpp/src/bitmask/null_mask.cu Outdated
jrhemstad
jrhemstad reviewed Feb 23, 2021
View reviewed changes
Comment thread cpp/src/bitmask/null_mask.cu
rwlee
rwlee commented Feb 23, 2021
View reviewed changes
Comment thread cpp/src/bitmask/null_mask.cu
@rwlee
rwlee requested a review from jrhemstad February 27, 2021 00:40
jrhemstad
jrhemstad reviewed Mar 1, 2021
View reviewed changes
Comment thread cpp/include/cudf/detail/null_mask.cuh Outdated
jrhemstad
jrhemstad reviewed Mar 1, 2021
View reviewed changes
Comment thread cpp/include/cudf/detail/null_mask.cuh Outdated
jrhemstad
jrhemstad reviewed Mar 1, 2021
View reviewed changes
Comment thread cpp/include/cudf/detail/null_mask.cuh Outdated
jrhemstad
jrhemstad reviewed Mar 1, 2021
View reviewed changes
Comment thread cpp/include/cudf/detail/null_mask.hpp Outdated
jrhemstad
jrhemstad reviewed Mar 1, 2021
View reviewed changes
Comment thread cpp/include/cudf/detail/null_mask.hpp Outdated
jrhemstad
jrhemstad reviewed Mar 1, 2021
View reviewed changes
Comment thread cpp/src/bitmask/null_mask.cu Outdated
jrhemstad
jrhemstad reviewed Mar 1, 2021
View reviewed changes
Comment thread cpp/src/bitmask/null_mask.cu Outdated
jlowe
jlowe reviewed Mar 2, 2021
View reviewed changes
Comment thread java/src/main/native/src/ColumnViewJni.cpp Outdated
jlowe
jlowe approved these changes Mar 2, 2021
View reviewed changes

@jlowe jlowe 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.

Java approval

@jlowe jlowe changed the title [REVIEW] bitmask_or implementation with bitmask refactor bitmask_or implementation with bitmask refactor Mar 2, 2021
jrhemstad
jrhemstad reviewed Mar 2, 2021
View reviewed changes
Comment thread cpp/include/cudf/detail/null_mask.cuh Outdated
jrhemstad
jrhemstad approved these changes Mar 2, 2021
View reviewed changes

@jrhemstad jrhemstad 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.

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

rwlee commented Mar 3, 2021

Copy link
Copy Markdown
Contributor Author

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

rwlee commented Mar 3, 2021

Copy link
Copy Markdown
Contributor Author

Rerun tests

@rwlee

rwlee commented Mar 4, 2021

Copy link
Copy Markdown
Contributor Author

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

rwlee commented Mar 8, 2021

Copy link
Copy Markdown
Contributor Author

@gpucibot merge

@rapids-bot
rapids-bot Bot merged commit 3480e2e into NVIDIA:branch-0.19 Mar 8, 2021
@rwlee rwlee mentioned this pull request Mar 8, 2021
Merged
rapids-bot Bot pushed a commit that referenced this pull request Mar 9, 2021
@rwlee
Remove detail from device_span (#7533)
53929eb 
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
@rwlee @hyperbolic2346
bitmask_or implementation with bitmask refactor (NVIDIA#7406)
e6659ca 
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: NVIDIA#7406
hyperbolic2346 pushed a commit to hyperbolic2346/cudf that referenced this pull request Mar 25, 2021
@rwlee @hyperbolic2346
Remove detail from device_span (NVIDIA#7533)
0784a31 
Fix for issue caused by stale PR issue from NVIDIA#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: NVIDIA#7533
@bdice bdice mentioned this pull request Nov 9, 2021
Merged
3 tasks
@wence- wence- mentioned this pull request Jun 19, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidwendt davidwendt davidwendt left review comments
@jrhemstad jrhemstad jrhemstad approved these changes
@vuule vuule Awaiting requested review from vuule
+2 more reviewers
@jlowe jlowe jlowe approved these changes
@cwharris cwharris cwharris approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@rwlee @jlowe @cwharris @jrhemstad @davidwendt @kkraus14