Add `static_multimap::pair_contains` by PointKernel · Pull Request #175 · NVIDIA/cuCollections

PointKernel · 2022-06-05T17:26:38Z

Depends on #174

Contributes to #173

This PR adds static_multimap::pair_contains.

include/cuco/detail/static_multimap/device_view_impl.inl

ttnghia · 2022-06-09T22:44:02Z

include/cuco/detail/static_multimap/device_view_impl.inl

-    auto current_slot = initial_slot(g, k);
+    auto current_slot = [&]() {
+      if constexpr (is_pair_contains) { return initial_slot(g, element.first); }
+      if constexpr (not is_pair_contains) { return initial_slot(g, element); }


Similar to above.

And also all other places.

Now I recall, we need this to silence the compiler warnings with cuda-11.0/11.2. Reverting back to the if constexpr (not ...) expression.

include/cuco/detail/static_multimap/static_multimap.inl

include/cuco/static_multimap.cuh

ttnghia · 2022-06-10T18:42:46Z

I tested locally with my PR. Will work with this fix: 331b248.
This also needs more cleanup and fixing for docs.

ttnghia · 2022-06-14T18:56:38Z

include/cuco/static_multimap.cuh

+    __device__ __forceinline__ bool pair_contains(
+      cooperative_groups::thread_block_tile<ProbeSequence::cg_size> const& g,
+      value_type const& p,
+      PairEqual pair_equal = PairEqual{}) noexcept;


In addition, we also need to add the version of pair_contains here without the thread_block_tile parameter.

Why is that?

This is exposed to the users, right? So the users don't have that parameter.

Unlike single map, multimap needs to handle duplicates. That's the reason that we provide CG APIs only in multimap. Non-CG APIs are provided in single map but are also rarely used. Can you provide more details on your use case i.e. why would non-CG implementation is preferred over the CG ones?

So the users don't have that parameter.

users can create a CG e.g. https://godbolt.org/z/o6jYYv7Ys

include/cuco/static_multimap.cuh

…ntains

include/cuco/detail/probe_sequence_impl.cuh

include/cuco/static_multimap.cuh

tests/static_multimap/pair_function_test.cu

…param

ttnghia · 2022-06-15T18:01:49Z

Closes #173

Hey, since #173 now saying "Support pair_contains in static_map and static_multimap", this should not be enough to close that.

PointKernel · 2022-06-15T20:08:43Z

Closes #173

Hey, since #173 now saying "Support pair_contains in static_map and static_multimap", this should not be enough to close that.

I'm aware of that. Don't worry. :)

ttnghia · 2022-06-16T16:54:47Z

Confirm that the latest commit works with my code.

tests/static_multimap/pair_function_test.cu

…ntains

A (left) semi-join between the left and right tables returns a set of rows in the left table that has matching rows (i.e., compared equally) in the right table. As such, for each row in the left table, it needs to check if that row has a match in the right table. Such check is very generic and has applications in many other places, not just in semi-join. This PR exposes that check functionality as a new `cudf::detail::contains(table_view, table_view)` for internal usage. Closes #11037. Depends on: * NVIDIA/cuCollections#175 Authors: - Nghia Truong (https://github.com/ttnghia) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) - Yunsong Wang (https://github.com/PointKernel) URL: #11100

This PR adds the following APIs for set operations: * `lists::have_overlap` * `lists::intersect_distinct` * `lists::union_distinct` * `lists::difference_distinct` ### Name Convention Except for the first API (`lists::have_overlap`) that returns a boolean column, the suffix `_distinct` of the rest APIs denotes that their results will be lists columns in which all list rows have been post-processed to remove duplicates. As such, their results are actually "set" columns in which each row is a "set" of distinct elements. --- Depends on: * #10945 * #11017 * NVIDIA/cuCollections#175 * #11052 * #11118 * #11100 * #11149 Closes #10409. Authors: - Nghia Truong (https://github.com/ttnghia) - Yunsong Wang (https://github.com/PointKernel) Approvers: - Michael Wang (https://github.com/isVoid) - AJ Schmidt (https://github.com/ajschmidt8) - Bradley Dice (https://github.com/bdice) - Yunsong Wang (https://github.com/PointKernel) URL: #11043

Add multimap pair_contains

1366cfc

PointKernel added type: feature request New feature request helps: rapids Helps or needed by RAPIDS topic: static_multimap Issue related to the static_multimap labels Jun 5, 2022

PointKernel added 3 commits June 5, 2022 16:33

Reorder static_multimap::contains parameters

e815261

Fix a bug in pair_contains

ec0f19d

Add pair_contains unit test

8e88d14

ttnghia reviewed Jun 9, 2022

View reviewed changes

include/cuco/detail/static_multimap/device_view_impl.inl Show resolved Hide resolved

ttnghia reviewed Jun 9, 2022

View reviewed changes

include/cuco/detail/static_multimap/device_view_impl.inl Show resolved Hide resolved

ttnghia reviewed Jun 9, 2022

View reviewed changes

ttnghia reviewed Jun 10, 2022

View reviewed changes

include/cuco/detail/static_multimap/static_multimap.inl Outdated Show resolved Hide resolved

ttnghia reviewed Jun 10, 2022

View reviewed changes

include/cuco/detail/static_multimap/static_multimap.inl Outdated Show resolved Hide resolved

ttnghia reviewed Jun 10, 2022

View reviewed changes

include/cuco/static_multimap.cuh Outdated Show resolved Hide resolved

ttnghia reviewed Jun 10, 2022

View reviewed changes

include/cuco/static_multimap.cuh Outdated Show resolved Hide resolved

Cleanups

a0c178c

ttnghia mentioned this pull request Jun 14, 2022

Refactor semi_anti_join rapidsai/cudf#11100

Merged

ttnghia reviewed Jun 14, 2022

View reviewed changes

include/cuco/static_multimap.cuh Outdated Show resolved Hide resolved

jrhemstad reviewed Jun 14, 2022

View reviewed changes

include/cuco/static_multimap.cuh Outdated Show resolved Hide resolved

jrhemstad reviewed Jun 14, 2022

View reviewed changes

include/cuco/static_multimap.cuh Outdated Show resolved Hide resolved

PointKernel added 2 commits June 14, 2022 16:51

Merge remote-tracking branch 'upstream/dev' into add-multimap-pair-co…

9e07fc9

…ntains

Make pair_contains template + make contains APIs const

1657014

jrhemstad reviewed Jun 15, 2022

View reviewed changes

include/cuco/detail/probe_sequence_impl.cuh Outdated Show resolved Hide resolved

jrhemstad reviewed Jun 15, 2022

View reviewed changes

include/cuco/static_multimap.cuh Outdated Show resolved Hide resolved

jrhemstad reviewed Jun 15, 2022

View reviewed changes

tests/static_multimap/pair_function_test.cu Outdated Show resolved Hide resolved

Cleanups: add non-const initial_slot, update test and remove default …

e6a71ae

…param

PointKernel requested a review from jrhemstad June 15, 2022 14:51

PointKernel added the Needs Review Awaiting reviews before merging label Jun 15, 2022

Use if constexpr not to silence build warnings with else

32d0b17

PointKernel force-pushed the add-multimap-pair-contains branch from 469776f to 32d0b17 Compare June 15, 2022 15:52

Update docs

4618205

PointKernel mentioned this pull request Jun 21, 2022

Add static_multimap::pair_contains_if #181

Closed

ttnghia mentioned this pull request Jun 23, 2022

Support set operations rapidsai/cudf#11043

Merged

jrhemstad reviewed Jul 11, 2022

View reviewed changes

tests/static_multimap/pair_function_test.cu Outdated Show resolved Hide resolved

Use explicit iterator value type instead of auto

bfa1461

jrhemstad mentioned this pull request Jul 12, 2022

Add tests that verify cuCo algorithms work with Thrust proxy_references #188

Closed

jrhemstad approved these changes Jul 12, 2022

View reviewed changes

PointKernel added 2 commits July 12, 2022 17:31

Merge remote-tracking branch 'upstream/dev' into add-multimap-pair-co…

107ee9b

…ntains

Fix Doxygen check warnings

0462bea

PointKernel merged commit 3a49fc7 into NVIDIA:dev Jul 12, 2022

PointKernel deleted the add-multimap-pair-contains branch July 12, 2022 22:20

PointKernel mentioned this pull request Jul 20, 2022

Temporarily reverse semi-anti-join implementation rapidsai/cudf#11310

Closed

Conversation

PointKernel commented Jun 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ttnghia commented Jun 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ttnghia commented Jun 15, 2022

Uh oh!

PointKernel commented Jun 15, 2022

Uh oh!

ttnghia commented Jun 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

PointKernel commented Jun 5, 2022 •

edited

Loading

ttnghia commented Jun 10, 2022 •

edited

Loading

ttnghia commented Jun 16, 2022 •

edited

Loading