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

[FEA]: Replace cuco::detail::MurmurHash3_32 with a proper public API #3692

Closed
2 tasks done
PointKernel opened this issue Jul 5, 2023 · 0 comments · Fixed by #3694
Closed
2 tasks done

[FEA]: Replace cuco::detail::MurmurHash3_32 with a proper public API #3692

PointKernel opened this issue Jul 5, 2023 · 0 comments · Fixed by #3694
Assignees
Labels
feature request New feature or request

Comments

@PointKernel
Copy link
Member

Is this a new feature, an improvement, or a change to existing functionality?

Improvement

How would you describe the priority of this feature request

High

Please provide a clear description of problem this feature solves

NVIDIA/cuCollections#310 introduced a breaking change that the content of cuco/detail/hash_functions.cuh has been reorganized to a different place. As a result, many header inclusions in cugraph will crash, e.g.:

#include <cuco/detail/hash_functions.cuh>

This will be an issue if we want to bump the cuco version in rapids-cmake.

Describe your ideal solution

Include the public hasher header and choose a proper hash function in cugraph, e.g.:

#include <cuco/hash_functions.cuh>

...

void func() {
  auto hasher = cuco::murmurhash3_32<vertex_t>{};
}

Note cuco provides new 32-bit hasher options like cuco::xxhash_32 which is proved to be more efficient than murmurhash. Worth testing in cugraph.

Describe any alternatives you have considered

Use cuco default hasher: https://github.com/NVIDIA/cuCollections/blob/806aa8051ba933c758636586e2c34487282465f0/include/cuco/hash_functions.cuh#L76

Additional context

No response

Code of Conduct

  • I agree to follow cuGraph's Code of Conduct
  • I have searched the open feature requests and have found no duplicates for this feature request
@PointKernel PointKernel added ? - Needs Triage Need team to review and classify feature request New feature or request labels Jul 5, 2023
@seunghwak seunghwak linked a pull request Jul 5, 2023 that will close this issue
@BradReesWork BradReesWork removed the ? - Needs Triage Need team to review and classify label Jul 6, 2023
@rapids-bot rapids-bot bot closed this as completed in #3694 Jul 6, 2023
rapids-bot bot pushed a commit to rapidsai/rapids-cmake that referenced this issue Jul 12, 2023
This PR bump the cuco version to the latest.

Opened rapidsai/cudf#13665 to verify it won't break cudf.
Opened rapidsai/raft#1641 to verify it won't break raft.

rapidsai/cugraph#3692 to be addressed once this PR is merged.

Authors:
  - Yunsong Wang (https://github.com/PointKernel)

Approvers:
  - Robert Maynard (https://github.com/robertmaynard)

URL: #435
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants