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

Support any hash constructor in hash_test.cu::check_hash_result #520

Open
esoha-nvidia opened this issue Jun 25, 2024 · 1 comment
Open
Labels
good first issue Good for newcomers type: improvement Improvement / enhancement to an existing function

Comments

@esoha-nvidia
Copy link

          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 #514 (comment)

@sleeepyjack sleeepyjack added good first issue Good for newcomers type: improvement Improvement / enhancement to an existing function labels Jun 25, 2024
@sleeepyjack
Copy link
Collaborator

I like the idea. We'll incorporate it into the test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers type: improvement Improvement / enhancement to an existing function
Projects
None yet
Development

No branches or pull requests

2 participants