Skip to content

Commit

Permalink
Merge pull request #967 from aprokop/access_traits_proliferation_prim…
Browse files Browse the repository at this point in the history 
…itives

Cut down on the number of AccessTraits
  • Loading branch information
@aprokop
aprokop committed Dec 7, 2023
2 parents 851e915 + e70bd45 commit 5a8d82e
Show file tree
Hide file tree
Showing 9 changed files with 130 additions and 138 deletions.
19 changes: 9 additions & 10 deletions benchmarks/dbscan/ArborX_DBSCANVerification.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,26 +298,25 @@ bool verifyDBSCAN(ExecutionSpace exec_space, Primitives const &primitives,

static_assert(Kokkos::is_view<LabelsView>{});

using Access = AccessTraits<Primitives, PrimitivesTag>;
using MemorySpace = typename Access::memory_space;
using Points = Details::AccessValues<Primitives>;
using MemorySpace = typename Points::memory_space;

static_assert(std::is_same<typename LabelsView::value_type, int>{});
static_assert(std::is_same<typename LabelsView::memory_space, MemorySpace>{});

ARBORX_ASSERT(eps > 0);
ARBORX_ASSERT(core_min_size >= 2);

using Point = typename Details::AccessTraitsHelper<Access>::type;
Points points{primitives};

using Point = typename Points::value_type;
static_assert(GeometryTraits::is_point<Point>{});
constexpr int dim = GeometryTraits::dimension_v<Point>;
using Box = ExperimentalHyperGeometry::Box<dim>;

ArborX::BasicBoundingVolumeHierarchy<MemorySpace,
ArborX::Details::PairIndexVolume<Box>>
bvh(exec_space,
ArborX::Details::LegacyValues<Primitives, Box>{primitives});
ArborX::Details::PairIndexVolume<Point>>
bvh(exec_space, ArborX::Details::LegacyValues<Points, Point>{points});

auto const predicates =
Details::PrimitivesWithRadius<Primitives>{primitives, eps};
auto const predicates = Details::PrimitivesWithRadius<Points>{points, eps};

Kokkos::View<int *, MemorySpace> indices("ArborX::DBSCAN::indices", 0);
Kokkos::View<int *, MemorySpace> offset("ArborX::DBSCAN::offset", 0);
Expand Down
17 changes: 9 additions & 8 deletions src/ArborX_BruteForce.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,11 @@ class BruteForce

template <typename MemorySpace, typename Value, typename IndexableGetter,
typename BoundingVolume>
template <typename ExecutionSpace, typename Values>
template <typename ExecutionSpace, typename UserValues>
BasicBruteForce<MemorySpace, Value, IndexableGetter, BoundingVolume>::
BasicBruteForce(ExecutionSpace const &space, Values const &user_values,
BasicBruteForce(ExecutionSpace const &space, UserValues const &user_values,
IndexableGetter const &indexable_getter)
: _size(AccessTraits<Values, PrimitivesTag>::size(user_values))
: _size(AccessTraits<UserValues, PrimitivesTag>::size(user_values))
, _values(Kokkos::view_alloc(space, Kokkos::WithoutInitializing,
"ArborX::BruteForce::values"),
_size)
Expand All @@ -184,10 +184,13 @@ BasicBruteForce<MemorySpace, Value, IndexableGetter, BoundingVolume>::
static_assert(
KokkosExt::is_accessible_from<MemorySpace, ExecutionSpace>::value);
// FIXME redo with RangeTraits
Details::check_valid_access_traits<Values>(
Details::check_valid_access_traits<UserValues>(
PrimitivesTag{}, user_values, Details::DoNotCheckGetReturnType());
using Access = AccessTraits<Values, PrimitivesTag>;
static_assert(KokkosExt::is_accessible_from<typename Access::memory_space,

using Values = Details::AccessValues<UserValues>;
Values values{user_values};

static_assert(KokkosExt::is_accessible_from<typename Values::memory_space,
ExecutionSpace>::value,
"Values must be accessible from the execution space");

Expand All @@ -198,8 +201,6 @@ BasicBruteForce<MemorySpace, Value, IndexableGetter, BoundingVolume>::
return;
}

Details::AccessValues<Values> values{user_values};

Details::BruteForceImpl::initializeBoundingVolumesAndReduceBoundsOfTheScene(
space, values, _indexable_getter, _values, _bounds);
}
Expand Down
122 changes: 50 additions & 72 deletions src/ArborX_DBSCAN.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,53 +83,33 @@ struct PrimitivesWithRadiusReorderedAndFiltered

// Mixed primitives consist of a set of boxes corresponding to dense cells,
// followed by boxes corresponding to points in non-dense cells.
template <typename PointPrimitives, typename DenseCellOffsets,
typename CellIndices, typename Permutation>
template <typename Points, typename DenseCellOffsets, typename CellIndices,
typename Permutation>
struct MixedBoxPrimitives
{
PointPrimitives _point_primitives;
CartesianGrid<GeometryTraits::dimension_v<typename AccessTraitsHelper<
AccessTraits<PointPrimitives, PrimitivesTag>>::type>>
_grid;
Points _points;
CartesianGrid<GeometryTraits::dimension_v<typename Points::value_type>> _grid;
DenseCellOffsets _dense_cell_offsets;
int _num_points_in_dense_cells; // to avoid lastElement() in AccessTraits
CellIndices _sorted_cell_indices;
Permutation _permute;
};

template <typename Primitives>
struct PrimitivesIndexables
{
Primitives _primitives;

using Access = AccessTraits<Primitives, PrimitivesTag>;
using memory_space = typename Access::memory_space;

KOKKOS_FUNCTION decltype(auto) operator()(int i) const
{
return Access::get(_primitives, i);
}

KOKKOS_FUNCTION auto size() const { return Access::size(_primitives); }
};

} // namespace Details

template <typename Primitives>
struct AccessTraits<Details::PrimitivesWithRadius<Primitives>, PredicatesTag>
{
using PrimitivesAccess = AccessTraits<Primitives, PrimitivesTag>;

using memory_space = typename PrimitivesAccess::memory_space;
using memory_space = typename Primitives::memory_space;
using Predicates = Details::PrimitivesWithRadius<Primitives>;

static KOKKOS_FUNCTION size_t size(Predicates const &w)
{
return PrimitivesAccess::size(w._primitives);
return w._primitives.size();
}
static KOKKOS_FUNCTION auto get(Predicates const &w, size_t i)
{
auto const &point = PrimitivesAccess::get(w._primitives, i);
auto const &point = w._primitives(i);
constexpr int dim =
GeometryTraits::dimension_v<std::decay_t<decltype(point)>>;
// FIXME reinterpret_cast is dangerous here if access traits return user
Expand All @@ -147,9 +127,7 @@ struct AccessTraits<Details::PrimitivesWithRadiusReorderedAndFiltered<
Primitives, PermuteFilter>,
PredicatesTag>
{
using PrimitivesAccess = AccessTraits<Primitives, PrimitivesTag>;

using memory_space = typename PrimitivesAccess::memory_space;
using memory_space = typename Primitives::memory_space;
using Predicates =
Details::PrimitivesWithRadiusReorderedAndFiltered<Primitives,
PermuteFilter>;
Expand All @@ -161,7 +139,7 @@ struct AccessTraits<Details::PrimitivesWithRadiusReorderedAndFiltered<
static KOKKOS_FUNCTION auto get(Predicates const &w, size_t i)
{
int index = w._filter(i);
auto const &point = PrimitivesAccess::get(w._primitives, index);
auto const &point = w._primitives(index);
constexpr int dim =
GeometryTraits::dimension_v<std::decay_t<decltype(point)>>;
// FIXME reinterpret_cast is dangerous here if access traits return user
Expand All @@ -174,13 +152,13 @@ struct AccessTraits<Details::PrimitivesWithRadiusReorderedAndFiltered<
}
};

template <typename PointPrimitives, typename MixedOffsets, typename CellIndices,
template <typename Points, typename MixedOffsets, typename CellIndices,
typename Permutation>
struct AccessTraits<Details::MixedBoxPrimitives<PointPrimitives, MixedOffsets,
CellIndices, Permutation>,
ArborX::PrimitivesTag>
struct AccessTraits<
Details::MixedBoxPrimitives<Points, MixedOffsets, CellIndices, Permutation>,
ArborX::PrimitivesTag>
{
using Primitives = Details::MixedBoxPrimitives<PointPrimitives, MixedOffsets,
using Primitives = Details::MixedBoxPrimitives<Points, MixedOffsets,
CellIndices, Permutation>;
static KOKKOS_FUNCTION std::size_t size(Primitives const &w)
{
Expand Down Expand Up @@ -209,11 +187,9 @@ struct AccessTraits<Details::MixedBoxPrimitives<PointPrimitives, MixedOffsets,
// For a primitive corresponding to a point in a non-dense cell, use that
// point. But first, figure out its index, which requires some
// computations.
using Access = AccessTraits<PointPrimitives, PrimitivesTag>;

i = (i - num_dense_primitives) + w._num_points_in_dense_cells;

auto const &point = Access::get(w._point_primitives, w._permute(i));
auto const &point = w._points(w._permute(i));
constexpr int dim =
GeometryTraits::dimension_v<std::decay_t<decltype(point)>>;
// FIXME reinterpret_cast is dangerous here if access traits return user
Expand Down Expand Up @@ -263,8 +239,8 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,
{
Kokkos::Profiling::pushRegion("ArborX::DBSCAN");

using Access = AccessTraits<Primitives, PrimitivesTag>;
using MemorySpace = typename Access::memory_space;
using Points = Details::AccessValues<Primitives>;
using MemorySpace = typename Points::memory_space;

static_assert(
KokkosExt::is_accessible_from<MemorySpace, ExecutionSpace>::value,
Expand All @@ -281,7 +257,7 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,
using UnionFind = Details::UnionFind<MemorySpace>;
#endif

using Point = typename Details::AccessTraitsHelper<Access>::type;
using Point = typename Points::value_type;
static_assert(GeometryTraits::is_point<Point>{});
constexpr int dim = GeometryTraits::dimension_v<Point>;
using Box = ExperimentalHyperGeometry::Box<dim>;
Expand All @@ -290,7 +266,8 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,

bool const verbose = parameters._verbose;

int const n = Access::size(primitives);
Points points{primitives};
int const n = points.size();

Kokkos::View<int *, MemorySpace> num_neigh("ArborX::DBSCAN::num_neighbors",
0);
Expand All @@ -307,7 +284,7 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,
Kokkos::Profiling::pushRegion("ArborX::DBSCAN::tree_construction");
ArborX::BasicBoundingVolumeHierarchy<MemorySpace,
Details::PairIndexVolume<Point>>
bvh(exec_space, Details::LegacyValues<Primitives, Point>{primitives});
bvh(exec_space, Details::LegacyValues<Points, Point>{points});
Kokkos::Profiling::popRegion();

Kokkos::Profiling::pushRegion("ArborX::DBSCAN::clusters");
Expand All @@ -333,7 +310,7 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,
else
{
auto const predicates =
Details::PrimitivesWithRadius<Primitives>{primitives, eps};
Details::PrimitivesWithRadius<Points>{points, eps};

// Determine core points
Kokkos::Profiling::pushRegion("ArborX::DBSCAN::clusters::num_neigh");
Expand Down Expand Up @@ -368,16 +345,17 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,
Kokkos::Profiling::pushRegion("ArborX::DBSCAN::dense_cells");
Box bounds;
Details::TreeConstruction::calculateBoundingBoxOfTheScene(
exec_space, Details::PrimitivesIndexables<Primitives>{primitives},
exec_space,
Details::Indexables<Points, Details::DefaultIndexableGetter>{
points, Details::DefaultIndexableGetter{}},
bounds);

// The cell length is chosen to be eps/sqrt(dimension), so that any two
// points within the same cell are within eps distance of each other.
float const h = eps / std::sqrt(dim);
Details::CartesianGrid<dim> const grid(bounds, h);

auto cell_indices =
Details::computeCellIndices(exec_space, primitives, grid);
auto cell_indices = Details::computeCellIndices(exec_space, points, grid);

auto permute = Details::sortObjects(exec_space, cell_indices);
auto &sorted_cell_indices = cell_indices; // alias
Expand Down Expand Up @@ -428,11 +406,14 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,

// Build the tree
Kokkos::Profiling::pushRegion("ArborX::DBSCAN::tree_construction");
Details::MixedBoxPrimitives<Primitives, decltype(dense_cell_offsets),
Details::MixedBoxPrimitives<Points, decltype(dense_cell_offsets),
decltype(cell_indices), decltype(permute)>
mixed_primitives{primitives, grid,
dense_cell_offsets, num_points_in_dense_cells,
sorted_cell_indices, permute};
mixed_primitives{points,
grid,
dense_cell_offsets,
num_points_in_dense_cells,
sorted_cell_indices,
permute};

ArborX::BasicBoundingVolumeHierarchy<MemorySpace,
Details::PairIndexVolume<Box>>
Expand All @@ -449,14 +430,13 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,
using CorePoints = Details::CCSCorePoints;
Kokkos::Profiling::pushRegion("ArborX::DBSCAN::clusters::query");
auto const predicates =
Details::PrimitivesWithRadius<Primitives>{primitives, eps};
bvh.query(
exec_space, predicates,
Details::FDBSCANDenseBoxCallback<UnionFind, CorePoints, Primitives,
decltype(dense_cell_offsets),
decltype(permute)>{
labels, CorePoints{}, primitives, dense_cell_offsets, exec_space,
permute, eps});
Details::PrimitivesWithRadius<Points>{points, eps};
bvh.query(exec_space, predicates,
Details::FDBSCANDenseBoxCallback<UnionFind, CorePoints, Points,
decltype(dense_cell_offsets),
decltype(permute)>{
labels, CorePoints{}, points, dense_cell_offsets,
exec_space, permute, eps});
Kokkos::Profiling::popRegion();
}
else
Expand All @@ -477,13 +457,12 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,

auto const sparse_predicates =
Details::PrimitivesWithRadiusReorderedAndFiltered<
Primitives, decltype(sparse_permute)>{primitives, eps,
sparse_permute};
Points, decltype(sparse_permute)>{points, eps, sparse_permute};
bvh.query(exec_space, sparse_predicates,
Details::CountUpToN_DenseBox<MemorySpace, Primitives,
Details::CountUpToN_DenseBox<MemorySpace, Points,
decltype(dense_cell_offsets),
decltype(permute)>(
num_neigh, primitives, dense_cell_offsets, permute,
num_neigh, points, dense_cell_offsets, permute,
core_min_size, eps, core_min_size));
Kokkos::Profiling::popRegion();

Expand All @@ -492,14 +471,13 @@ dbscan(ExecutionSpace const &exec_space, Primitives const &primitives,
// Perform the queries and build clusters through callback
Kokkos::Profiling::pushRegion("ArborX::DBSCAN::clusters::query");
auto const predicates =
Details::PrimitivesWithRadius<Primitives>{primitives, eps};
bvh.query(
exec_space, predicates,
Details::FDBSCANDenseBoxCallback<UnionFind, CorePoints, Primitives,
decltype(dense_cell_offsets),
decltype(permute)>{
labels, CorePoints{num_neigh, core_min_size}, primitives,
dense_cell_offsets, exec_space, permute, eps});
Details::PrimitivesWithRadius<Points>{points, eps};
bvh.query(exec_space, predicates,
Details::FDBSCANDenseBoxCallback<UnionFind, CorePoints, Points,
decltype(dense_cell_offsets),
decltype(permute)>{
labels, CorePoints{num_neigh, core_min_size}, points,
dense_cell_offsets, exec_space, permute, eps});
Kokkos::Profiling::popRegion();
}
}
Expand Down
22 changes: 12 additions & 10 deletions src/ArborX_LinearBVH.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,14 +232,15 @@ using BVH = BoundingVolumeHierarchy<MemorySpace>;

template <typename MemorySpace, typename Value, typename IndexableGetter,
typename BoundingVolume>
template <typename ExecutionSpace, typename Values, typename SpaceFillingCurve>
template <typename ExecutionSpace, typename UserValues,
typename SpaceFillingCurve>
BasicBoundingVolumeHierarchy<MemorySpace, Value, IndexableGetter,
BoundingVolume>::
BasicBoundingVolumeHierarchy(ExecutionSpace const &space,
Values const &user_values,
UserValues const &user_values,
IndexableGetter const &indexable_getter,
SpaceFillingCurve const &curve)
: _size(AccessTraits<Values, PrimitivesTag>::size(user_values))
: _size(AccessTraits<UserValues, PrimitivesTag>::size(user_values))
, _leaf_nodes(Kokkos::view_alloc(space, Kokkos::WithoutInitializing,
"ArborX::BVH::leaf_nodes"),
_size)
Expand All @@ -251,10 +252,13 @@ BasicBoundingVolumeHierarchy<MemorySpace, Value, IndexableGetter,
static_assert(
KokkosExt::is_accessible_from<MemorySpace, ExecutionSpace>::value);
// FIXME redo with RangeTraits
Details::check_valid_access_traits<Values>(
Details::check_valid_access_traits<UserValues>(
PrimitivesTag{}, user_values, Details::DoNotCheckGetReturnType());
using Access = AccessTraits<Values, PrimitivesTag>;
static_assert(KokkosExt::is_accessible_from<typename Access::memory_space,

using Values = Details::AccessValues<UserValues>;
Values values{user_values};

static_assert(KokkosExt::is_accessible_from<typename Values::memory_space,
ExecutionSpace>::value,
"Values must be accessible from the execution space");

Expand All @@ -269,17 +273,15 @@ BasicBoundingVolumeHierarchy<MemorySpace, Value, IndexableGetter,
return;
}

Details::AccessValues<Values> values{user_values};

if (size() == 1)
{
Details::TreeConstruction::initializeSingleLeafTree(
space, values, _indexable_getter, _leaf_nodes, _bounds);
return;
}

Details::Indexables<decltype(values), IndexableGetter> indexables{
values, indexable_getter};
Details::Indexables<Values, IndexableGetter> indexables{values,
indexable_getter};

Kokkos::Profiling::pushRegion(
"ArborX::BVH::BVH::calculate_scene_bounding_box");
Expand Down

0 comments on commit 5a8d82e

Please sign in to comment.