Allow CUDA CCA algorithm to output cluster info #930
Conversation
stephenswat
added
feature
|
Tagging @paradajzblond as an interested party. |
device/cuda/include/traccc/cuda/clusterization/clusterization_algorithm.hpp
Show resolved
Hide resolved
device/cuda/include/traccc/cuda/clusterization/clusterization_algorithm.hpp
Outdated
Show resolved
Hide resolved
device/cuda/include/traccc/cuda/clusterization/clusterization_algorithm.hpp
Outdated
Show resolved
Hide resolved
stephenswat
force-pushed
the
feat/cca_cluster_data
branch
2 times, most recently
from
2eaa3d6 to
372a084
Compare
stephenswat
force-pushed
the
feat/cca_cluster_data
branch
2 times, most recently
from
1ad193a to
3b6c34d
Compare
|
All good to go! |
stephenswat
force-pushed
the
feat/cca_cluster_data
branch
2 times, most recently
from
94b755b to
f1eaaee
Compare
There was a problem hiding this comment.
To put it here as well, I personally would be a lot happier if the new function arguments would also be given to the functions via vecmem views. And in fact I plan to update the code myself to that formalism when upgrading the Alpaka and SYCL algorithms.
But other than that, only some small requests for updating the documentation. And don't forget to add the missing .cu file!
device/common/include/traccc/clusterization/device/aggregate_cluster.hpp
Outdated
Show resolved
Hide resolved
device/common/include/traccc/clusterization/device/impl/aggregate_cluster.ipp
Outdated
Show resolved
Hide resolved
Please feel free to try, but I will just warn you again that you are wasting your time. If you try this, you will notice that using vecmem views here is a lot of effort, adds a lot of complication, and yields no benefits. |
stephenswat
force-pushed
the
feat/cca_cluster_data
branch
2 times, most recently
from
50cef21 to
213979c
Compare
This commit allows the CUDA CCA algorithm (and, in the future, the SYCL and Alpaka equivalents) to output cluster information in the form of a disjoint set array. The design is such that the existing API is unchanged, but that extra function calls become available.
stephenswat
force-pushed
the
feat/cca_cluster_data
branch
from
213979c to
c1b95a5
Compare
|
| if (cluster_size.has_value()) { | ||
| (*cluster_size).get() = tmp_cluster_size; | ||
| } |
There was a problem hiding this comment.
😕 All I wrote was to try to get rid of this if-block. As long as the block is still here, the new function argument is completely useless. Please either
- try the formalism that I suggested, and see what it does to the compiled code;
- or go back to using a pointer.
This current version is the worst of all worlds. 😦
| template <device::concepts::thread_id1 thread_id_t> | ||
| TRACCC_HOST_DEVICE void reify_cluster_data( | ||
| const thread_id_t& thread_id, unsigned int* disjoint_set_ptr, | ||
| unsigned int num_cells, | ||
| traccc::edm::silicon_cluster_collection::view cluster_view) { |
There was a problem hiding this comment.
Please provide some function documentation.
|
... |
This commit allows the CUDA CCA algorithm (and, in the future, the SYCL and Alpaka equivalents) to output cluster information in the form of a disjoint set array. The design is such that the existing API is unchanged, but that extra function calls become available.