Skip to content

Commit

Permalink
Merge pull request #525 from aprokop/dbscan_bugfix
Browse files Browse the repository at this point in the history
  • Loading branch information
@aprokop
aprokop authored May 28, 2021
2 parents ba1a14f + 6d59299 commit fc78f08
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 10 deletions.
32 changes: 30 additions & 2 deletions examples/dbscan/ArborX_DBSCANVerification.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,35 @@ namespace ArborX
namespace Details
{

// Check that core points have nonnegative indices
template <typename ExecutionSpace, typename IndicesView, typename OffsetView,
typename LabelsView>
bool verifyCorePointsNonnegativeIndex(ExecutionSpace const &exec_space,
IndicesView /*indices*/,
OffsetView offset, LabelsView labels,
int core_min_size)
{
int n = labels.size();

int num_incorrect = 0;
Kokkos::parallel_reduce(
"ArborX::DBSCAN::verify_core_points_nonnegative",
Kokkos::RangePolicy<ExecutionSpace>(exec_space, 0, n),
KOKKOS_LAMBDA(int i, int &update) {
bool self_is_core_point = (offset(i + 1) - offset(i) >= core_min_size);
if (self_is_core_point && labels(i) < 0)
{
#ifndef __SYCL_DEVICE_ONLY__
printf("Core point is marked as noise: %d [%d]\n", i, labels(i));
#endif
update++;
}
},
num_incorrect);
return (num_incorrect == 0);
}

// Check that connected core points have same cluster indices
// NOTE: if core_min_size = 2, all points are core points
template <typename ExecutionSpace, typename IndicesView, typename OffsetView,
typename LabelsView>
bool verifyConnectedCorePointsShareIndex(ExecutionSpace const &exec_space,
Expand Down Expand Up @@ -242,7 +269,8 @@ bool verifyClusters(ExecutionSpace const &exec_space, IndicesView indices,
using Verify = bool (*)(ExecutionSpace const &, IndicesView, OffsetView,
LabelsView, int);

for (auto verify : {static_cast<Verify>(verifyConnectedCorePointsShareIndex),
for (auto verify : {static_cast<Verify>(verifyCorePointsNonnegativeIndex),
static_cast<Verify>(verifyConnectedCorePointsShareIndex),
static_cast<Verify>(verifyBoundaryAndNoisePoints),
static_cast<Verify>(verifyClustersAreUnique)})
{
Expand Down
37 changes: 29 additions & 8 deletions src/ArborX_DBSCAN.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,9 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,
timer_start(timer);
Kokkos::Profiling::pushRegion("ArborX::dbscan::clusters");

Kokkos::View<int *, MemorySpace> num_neigh("ArborX::dbscan::num_neighbors",
0);

Kokkos::View<int *, MemorySpace> labels(
Kokkos::ViewAllocateWithoutInitializing("ArborX::DBSCAN::labels"), n);
ArborX::iota(exec_space, labels);
Expand All @@ -160,8 +163,7 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,
Kokkos::Timer timer_local;
timer_start(timer_local);
Kokkos::Profiling::pushRegion("ArborX::dbscan::clusters::num_neigh");
Kokkos::View<int *, MemorySpace> num_neigh("ArborX::dbscan::num_neighbors",
n);
Kokkos::resize(num_neigh, n);
bvh.query(exec_space, predicates,
Details::CountUpToN<MemorySpace>{num_neigh, core_min_size});
Kokkos::Profiling::popRegion();
Expand Down Expand Up @@ -203,12 +205,31 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,

Kokkos::atomic_fetch_add(&cluster_sizes(labels(i)), 1);
});
Kokkos::parallel_for("ArborX::dbscan::mark_noise",
Kokkos::RangePolicy<ExecutionSpace>(exec_space, 0, n),
KOKKOS_LAMBDA(int const i) {
if (cluster_sizes(labels(i)) == 1)
labels(i) = -1;
});
if (is_special_case)
{
// Ideally, this kernel would have had the exactly same form as in the
// else() clause. But there's no available valid is_core() for use here:
// - CCSCorePoints cannot be used as it always returns true, which is OK
// inside the callback, but not here
// - DBSCANCorePoints cannot be used either as num_neigh is not initialized
// in the special case.
Kokkos::parallel_for("ArborX::dbscan::mark_noise",
Kokkos::RangePolicy<ExecutionSpace>(exec_space, 0, n),
KOKKOS_LAMBDA(int const i) {
if (cluster_sizes(labels(i)) == 1)
labels(i) = -1;
});
}
else
{
DBSCAN::DBSCANCorePoints<MemorySpace> is_core{num_neigh, core_min_size};
Kokkos::parallel_for("ArborX::dbscan::mark_noise",
Kokkos::RangePolicy<ExecutionSpace>(exec_space, 0, n),
KOKKOS_LAMBDA(int const i) {
if (cluster_sizes(labels(i)) == 1 && !is_core(i))
labels(i) = -1;
});
}
Kokkos::Profiling::popRegion();
elapsed["query+cluster"] = timer_seconds(timer);

Expand Down
56 changes: 56 additions & 0 deletions test/tstDBSCAN.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,36 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(dbscan_verifier, DeviceType, ARBORX_DEVICE_TYPES)
!verifyDBSCAN(space, points, 1, 4,
buildView<DeviceType, int>({5, 5, 5, 5, 5, 5, 5})));
}

{
// check where a core point is connected to only boundary points, but which
// are stripped by a second core point

// o - core, x - border
// -1 0 1 2
// ----------
// 2 | x o x
// 1 | x x
// 0 | o x o
// -1 | x x
// -2 | x o x
// clang-format off
auto points = buildView<DeviceType, Point>({
{0, -2, 0}, {-1, -2, 0}, {1, -2, 0}, {0, -1, 0}, // bottom
{0, 2, 0}, {-1, 2, 0}, {1, 2, 0}, {0, 1, 0}, // top
{2, 0, 0}, {2, -1, 0}, {2, 1, 0}, {1, 0, 0}, // right
{0, 0, 0} // stripped core
});
// clang-format on

BOOST_TEST(verifyDBSCAN(
space, points, 1, 4,
buildView<DeviceType, int>({0, 0, 0, 0, 1, 1, 1, 1, 2, 2, 2, 2, 5})));
// make sure the stripped core is not marked as noise
BOOST_TEST(!verifyDBSCAN(
space, points, 1, 4,
buildView<DeviceType, int>({0, 0, 0, 0, 1, 1, 1, 1, 2, 2, 2, 2, -1})));
}
}

BOOST_AUTO_TEST_CASE_TEMPLATE(dbscan, DeviceType, ARBORX_DEVICE_TYPES)
Expand Down Expand Up @@ -198,6 +228,32 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(dbscan, DeviceType, ARBORX_DEVICE_TYPES)
BOOST_TEST(
verifyDBSCAN(space, points, 1.0, 4, dbscan(space, points, 1, 4)));
}

{
// check where a core point is connected to only boundary points, but which
// are stripped by a second core point

// o - core, x - border
// -1 0 1 2
// ----------
// 2 | x o x
// 1 | x x
// 0 | o x o
// -1 | x x
// -2 | x o x
// clang-format off
auto points = buildView<DeviceType, Point>({
{0, -2, 0}, {-1, -2, 0}, {1, -2, 0}, {0, -1, 0}, // bottom
{0, 2, 0}, {-1, 2, 0}, {1, 2, 0}, {0, 1, 0}, // top
{2, 0, 0}, {2, -1, 0}, {2, 1, 0}, {1, 0, 0}, // right
{0, 0, 0} // stripped core
});
// clang-format on

// This does *not* guarantee to trigger the issue, as it depends on the
// specific implementation and runtime. But it may.
BOOST_TEST(verifyDBSCAN(space, points, 1, 4, dbscan(space, points, 1, 4)));
}
}

BOOST_AUTO_TEST_SUITE_END()

0 comments on commit fc78f08

Please sign in to comment.