Add a DeviceHash class for GPU hashing#1583
Conversation
publixsubfan
added
Core
kennyweiss
left a comment
kennyweiss
left a comment
There was a problem hiding this comment.
Thanks @publixsubfan
It's be helpful to see how DeviceHash could be used w/ a composite type, e.g. a simple 3D-point type.
Please also update the RELEASE-NOTES
There was a problem hiding this comment.
Can you please add a test to show how one might extend this to new types, e.g. to a Point struct w/ x-, y- and z fields, or to an axom::NumericalArray ?
There was a problem hiding this comment.
My initial thought is that for user-provided types, we could ask users to create a template specialization of axom:::DeviceHash; if we wanted to provide a "default" hash for our internal types, we could specialize the helper function axom::detail::DeviceHashHelper.
There was a problem hiding this comment.
Thanks -- I don't think we need to add any for our internal types to this PR, just thought it'd be helpful to add a simple example to show how one might do it.
There was a problem hiding this comment.
I added a small test as an example of a user-defined hash specialization. The main caveat is that the DeviceHash specialization must be defined within the axom namespace.
| using result_type = axom::IndexType; | ||
| AXOM_HOST_DEVICE axom::IndexType operator()(T value) const | ||
| { | ||
| // Special case: -0.0 and 0.0 compare equal but have different byte representations. |
| using result_type = axom::IndexType; | ||
| AXOM_HOST_DEVICE axom::IndexType operator()(T* ptr) const | ||
| { | ||
| return reinterpret_cast<axom::IndexType>(ptr); |
There was a problem hiding this comment.
I recall there might have been an issue with the pointer-to-int static_cast losing precision, and triggering a warning. I'll take a look again.
There was a problem hiding this comment.
not a big deal. I was just curious if there was an issue with static_cast in this context.
| // Invocations of the hash function should be idempotent. | ||
| EXPECT_EQ(computed_hashes[i], device_hasher(things_to_hash[i])); | ||
|
|
||
| // Check that we don't have hash collisions with other values. |
There was a problem hiding this comment.
Thinking out loud: is there a good way to check whether there could be collisions with two arbitrary, but different, integral values?
|
|
||
| TEST(core_device_hash, hash_int) | ||
| { | ||
| axom::DeviceHash<int> device_hasher; |
There was a problem hiding this comment.
Is there any reason not to unify all these tests to a single routine that takes a generic container templated on its value type?
There was a problem hiding this comment.
I think the conversions between different types, as well as enum handling, might be a little ugly. Also, there is a special case for floating points that we have to test (DeviceHash(-0.0) == DeviceHash(0.0))
There was a problem hiding this comment.
understood. I didn't look at it too closely.
Arlie-Capps
left a comment
Arlie-Capps
left a comment
There was a problem hiding this comment.
Thank you, @publixsubfan .
fc206bb to
e48fd46
Compare
| namespace | ||
| { | ||
| template <typename T> | ||
| struct UserVector | ||
| { | ||
| T x, y, z; | ||
| }; | ||
| } // namespace | ||
|
|
||
| // Test that we can correctly specialize a device hash for a user-defined type. | ||
| namespace axom | ||
| { | ||
| template <typename T> | ||
| struct DeviceHash<UserVector<T>> | ||
| { | ||
| using argument_type = UserVector<T>; | ||
| using result_type = axom::IndexType; | ||
|
|
||
| AXOM_HOST_DEVICE axom::IndexType operator()(UserVector<T> value) const | ||
| { | ||
| // Copy byte representation over | ||
| constexpr int NWORDS = sizeof(UserVector<T>) / sizeof(int); | ||
| alignas(UserVector<T>) int bytes[NWORDS]; | ||
| *reinterpret_cast<UserVector<T> *>(bytes) = value; | ||
|
|
||
| axom::IndexType hash_result {}; | ||
| for(int i = 0; i < NWORDS; i++) | ||
| { | ||
| hash_result ^= (bytes[i] + 0x853c49e6); | ||
| } | ||
| return hash_result; | ||
| } | ||
| }; | ||
| } // namespace axom |
af34933 to
619a876
Compare
5 participants
Summary
axom::DeviceHashfor supporting hashing certain primitive types (integers, floating points, enums) on the GPUstd::hash, which is to return the identity of the hashed value (casted toaxom::IndexType)std::hash::operator ()