Introduce Value as a generic storage in a leaf node #871
Conversation
|
The performance seems to be OKish, with about 8% slowdown in the construction on Serial on Power9. |
| @@ -70,15 +70,15 @@ struct HappyTreeFriends | |||
| { | |||
| static_assert( | |||
| std::is_same_v<decltype(bvh._internal_nodes(0).bounding_volume), | |||
| decltype(bvh._leaf_nodes(0).bounding_volume)>); | |||
| return bvh._leaf_nodes(i).bounding_volume; | |||
| decltype(bvh._leaf_nodes(0).value.bounding_volume)>); | |||
There was a problem hiding this comment.
Do we still want the static assert here?
There was a problem hiding this comment.
For now. Do you have a better idea or place for it?
There was a problem hiding this comment.
I would have dropped it since this is not something that user input can break but I don't feel too strongly about it.
| BoundingVolume bounding_volume) noexcept | ||
| template <class Value> | ||
| KOKKOS_INLINE_FUNCTION constexpr LeafNode<Value> | ||
| makeLeafNode(Value value) noexcept |
There was a problem hiding this comment.
I don't like so much spelling the template parameter at the call site but I suppose this way you won't have to change the definition later.
| @@ -54,8 +55,7 @@ struct HalfTraversal | |||
| { | |||
| auto const predicate = | |||
| _get_predicate(HappyTreeFriends::getLeafBoundingVolume(_bvh, i)); | |||
| auto const leaf_permutation_i = | |||
| HappyTreeFriends::getLeafPermutationIndex(_bvh, i); | |||
| auto const leaf_permutation_i = HappyTreeFriends::getValue(_bvh, i).index; | |||
There was a problem hiding this comment.
Yuk. I don't have a better idea though.
There was a problem hiding this comment.
I agree, would be nice to just have getValue() here. If we ever figure out how to move away from storing (index, volume) together in the future, this would be possible. But right now, it comes with severe penalties.
|
Please comment on what changed? Did you just amend the suggested changes into your first commit? |
Yes, sorry. |
|
SYCL build out of space, otherwise good. |
No description provided.