Use thrust::identity as hash functions for byte pair encoding
#13665
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
libcudf
7 tasks
|
CMake changes will be reverted once rapidsai/rapids-cmake#435 is merged. |
PointKernel
changed the title
thrust::identity as hash functions for byte pair encoding
PointKernel
added
bug
davidwendt
approved these changes
Jul 7, 2023
nvdbaranec
reviewed
Jul 7, 2023
| @@ -119,7 +119,7 @@ std::unique_ptr<detail::merge_pairs_map_type> initialize_merge_pairs_map( | |||
|
|
|||
| merge_pairs_map->insert(iter, | |||
| iter + input.size(), | |||
| cuco::murmurhash3_32<cudf::hash_value_type>{}, | |||
| thrust::identity<cudf::hash_value_type>{}, | |||
There was a problem hiding this comment.
What's the nature of this change? Is it just that it's pointless to be initializing the map with an actual hashed value?
There was a problem hiding this comment.
That's correct. Previously, this was hashing a hashed value.
Also, so that the same hasher is used for both insert and find -- a bug that this fixes too.
nvdbaranec
approved these changes
Jul 10, 2023
harrism
approved these changes
Jul 10, 2023
|
/merge |
rapids-bot bot
pushed a commit
to rapidsai/rapids-cmake
that referenced
this pull request
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes a minor issue that distinct hash functions are used for
insertandfindin byte pair encoding. It also verifies that the latest changes in cuco won't break cudf.Checklist