Introduce new algorithm (FDBSCAN-DenseBox) for DBSCAN #508
Conversation
aprokop
added
performance
aprokop
force-pushed
the
dbscan_densebox
branch
2 times, most recently
from
1d518c9
to
c3af33b
Compare
|
Rebased on |
src/ArborX_DBSCAN.hpp
Outdated
| nx, ny, nz, bounds); | ||
| auto permute = Details::sortObjects(exec_space, cell_indices); | ||
|
|
||
| auto mixed_offsets = Details::computeMixedOffsets(exec_space, core_min_size, |
There was a problem hiding this comment.
Why "mixed" offsets? I don't understand the terminology.
There was a problem hiding this comment.
It's a bit hard to explain. Essentially, we want to construct a tree on a combination of individual points (belonging to non-dense cells) and boxes of dense cells. In this sense, the primitives are mixed. Mixed offsets is essentially offsets in the sorted cells indices array corresponding to mixed primitives. For example, if you have [a a b b b] cell indices, and minpts = 3, it will result in the mixed offsets [0, 1, 2, 5] mixed offsets. So, it is not guaranteed that the values of the cell indices in the interval [mixed_offsets(i), mixed_offsets(i+1)) are different from [mixed_offset(i+1), mixed_offsets(i+2)).
| // NOTE: This is pretty bad. A single thread will scan a linear range | ||
| // corresponding to a dense cell. There is no upper limit on the | ||
| // number of points in such a cell. In the pathological case, all | ||
| // points may be contained in a single cell, making this completely | ||
| // serial. Is there a way to do better? |
There was a problem hiding this comment.
Does this dominate the execution time? Is it a "it would be nice" or a "must improve"?
There was a problem hiding this comment.
I have no quantitative answer, and I think it falls under would be nice to improve if we knew how.
| sparse_predicates(Kokkos::ViewAllocateWithoutInitializing( | ||
| "ArborX::dbscan::sparse_predicates"), |
There was a problem hiding this comment.
Predicates corresponding only to points that are in non-dense cells.
aprokop
force-pushed
the
dbscan_densebox
branch
2 times, most recently
from
67b593c
to
1567d9e
Compare
|
@dalg24 I think I addressed all your comments. Please give it another look. |
|
I separated first three commits into #519 for ease of the review. Will rebase once that PR is merged. |
|
Putting Update: yep, that was it. |
|
OK, I updated the PR with what I think close to the final version. Would appreciate if you review it again. |
It is unnecessary, and would save some memory.
Co-authored-by: Damien L-G <dalg24@gmail.com>
|
Rebased on master, and added a commit addressing review comments. |
|
Build failure is not a failure, but warnings from a container build. |
No description provided.