NVIDIA / NVIDIA/cuCollections

Support any hash constructor in hash_test.cu::check_hash_result

Open
#520 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

good first issue type: improvement
Dominant language
Cuda
Stars
667
Forks
120
Avg merge
7d 5h
Merged PRs (30d)
4

Description

          I see why you made this: Because we have hash functions that don't take a seed.  However, we'll eventually have the `SigBitHash` that takes not a seed but rather a list or a range or whatever.

I think that it would be better to not write a million of these functions. The right way to do it is with a variadic template. Also, you could make that a different PR. The code would look like this:

template <typename Hash, typename... HashConstructorArgs>
static __host__ __device__ bool check_hash_result(typename Hash::argument_type const& key,
                                                  typename Hash::result_type expected,
                                                  HashConstructorArgs&&... hash_constructor_args) noexcept
{
  Hash h(std::forward<HashConstructorArgs>(hash_constructor_args)...);
  return (h(key) == expected);
}

The seed is now the last argument so you need to find all occurrences of check_hash_result in this file and make the seed the last argument.

After doing this change, the function will now be compatible with all hash construction.


If you want to read more about std::forward, these are good:

The short of it is that std::forward written this way with a "universal reference" makes it so that this code works well when:

  • you pass in something that has a name, like check_hash_result(my_key, my_expected, my_seed). (my_seed has a "name", it's a left-value aka l-value aka something that can be on the left side of an assignment)
  • you pass in something nameless, like check_hash_result(my_key, my_expected, 12345). (12345 has no "name", it's a right-value aka r-value aka something that can not be on the left side of an assignment statement)

When used like this, the compiler will correctly optimize if the value of the seed or whatever is passed in there needs to be copied or moved.

Originally posted by @esoha-nvidia in https://github.com/NVIDIA/cuCollections/pull/514#discussion_r1653534402

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Open hash_test.cu and locate check_hash_result plus every call site in the file. Review the proposed variadic constructor interface, move each seed argument to the end, and run the hash tests to confirm existing results still pass and construction works without a seed.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
testing
Issue type
Refactor
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.