Skip to content

Commit

Permalink
Reorganize cuSpatial headers (#1097)
Browse files Browse the repository at this point in the history
Fixes #1080 based on the discussion in #1087

-  Consolidate each major set of features within common headers, e.g. `distance.hpp/.cuh`, `intersection.hpp/.cuh`.
-  Consolidate point-in-polygon APIs / headers and DRY up implementation
- Rename some headers/APIS that were inconsistent across the header-only and column-based APIs, e.g. `points_in_spatial_window` vs. `points_in_range`.
- Update Cython where needed to account for above changes.

This PR does NOT yet reorganize source code.

Update: Compilation Time Results
---------------------------------------------

Compilation time is not changed appreciably by this PR (<0.5% faster).

This test was performed by disabling `sccache`, erasing everything in the build directory, configuring cmake (including building tests and benchmarks), and then running `time ninja`. My machine has a "AMD Ryzen 7 3700X 8-Core Processor", and I'm using Ninja's default parallelism.

#### Disabling `sccache` when using the cuSpatial devcontainer

```
(rapids) coder ➜ ~/cuspatial/ $ cd cpp/build/release
(rapids) coder ➜ ~/cuspatial/cpp/build/release $ rm -rf ./*
(rapids) coder ➜ ~/cuspatial/cpp/build/release $ unset RUSTC_WRAPPER
(rapids) coder ➜ ~/cuspatial/cpp/build/release $ unset CMAKE_C_COMPILER_LAUNCHER
(rapids) coder ➜ ~/cuspatial/cpp/build/release $ unset CMAKE_CXX_COMPILER_LAUNCHER
(rapids) coder ➜ ~/cuspatial/cpp/build/release $ unset CMAKE_CUDA_COMPILER_LAUNCHER
(rapids) coder ➜ ~/cuspatial/cpp/build/release $ cmake ~/cuspatial/cpp -GNinja -DBUILD_TESTS=ON -DBUILD_BENCHMARKS=ON
(rapids) coder ➜ ~/cuspatial/cpp/build/release $ time ninja
```

### This branch: 

```
[201/201] Linking CXX executable gtests/LINESTRING_INTERSECTION_TEST_EXP

real    10m42.845s
user    122m28.077s
sys     6m7.935s
```

### `branch-23.06`:

```
[202/202] Linking CXX executable gtests/LINESTRING_INTERSECTION_TEST_EXP

real    10m45.573s
user    122m52.896s
sys     6m9.357s
```

Authors:
  - Mark Harris (https://github.com/harrism)

Approvers:
  - Michael Wang (https://github.com/isVoid)
  - Paul Taylor (https://github.com/trxcllnt)
  - H. Thomson Comer (https://github.com/thomcom)

URL: #1097
  • Loading branch information
harrism authored May 2, 2023
1 parent 2d02073 commit 2509fce
Show file tree
Hide file tree
Showing 166 changed files with 1,780 additions and 2,471 deletions.
7 changes: 3 additions & 4 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,11 @@ add_library(cuspatial
src/join/quadtree_point_in_polygon.cu
src/join/quadtree_point_to_nearest_linestring.cu
src/join/quadtree_bbox_filtering.cu
src/spatial/linestring_bounding_boxes.cu
src/spatial/polygon_bounding_boxes.cu
src/spatial/pairwise_multipoint_equals_count.cu
src/spatial/polygon_bounding_box.cu
src/spatial/linestring_bounding_box.cu
src/spatial/point_in_polygon.cu
src/spatial/pairwise_point_in_polygon.cu
src/spatial_window/spatial_window.cu
src/spatial/points_in_range.cu
src/spatial/haversine.cu
src/spatial/hausdorff.cu
src/spatial/linestring_distance.cu
Expand Down
4 changes: 2 additions & 2 deletions cpp/benchmarks/hausdorff_benchmark.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020-2021, NVIDIA CORPORATION.
* Copyright (c) 2020-2023, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -14,7 +14,7 @@
* limitations under the License.
*/

#include <cuspatial/distance/hausdorff.hpp>
#include <cuspatial/distance.hpp>

#include <benchmarks/fixture/benchmark_fixture.hpp>
#include <benchmarks/synchronization/synchronization.hpp>
Expand Down
5 changes: 2 additions & 3 deletions cpp/benchmarks/pairwise_linestring_distance.cu
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, NVIDIA CORPORATION.
* Copyright (c) 2022-2023, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -17,10 +17,9 @@
#include <benchmarks/fixture/rmm_pool_raii.hpp>
#include <nvbench/nvbench.cuh>

#include <cuspatial/detail/iterator.hpp>
#include <cuspatial/distance.cuh>
#include <cuspatial/geometry/vec_2d.hpp>
#include <cuspatial/iterator_factory.cuh>
#include <cuspatial/linestring_distance.cuh>
#include <cuspatial/range/multilinestring_range.cuh>

#include <rmm/device_vector.hpp>
Expand Down
2 changes: 1 addition & 1 deletion cpp/benchmarks/points_in_range.cu
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@

#include <cuspatial_test/random.cuh>

#include <cuspatial/detail/iterator.hpp>
#include <cuspatial/error.hpp>
#include <cuspatial/geometry/vec_2d.hpp>
#include <cuspatial/iterator_factory.cuh>
#include <cuspatial/points_in_range.cuh>

#include <rmm/device_uvector.hpp>
Expand Down
5 changes: 3 additions & 2 deletions cpp/doxygen/developer_guide/DOCUMENTATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -475,8 +475,9 @@ from the [doxygen_groups.h](../include/doxygen_groups.h) file.

} // namespace cuspatial

You can also use the \@addtogroup with a `@{ ... @}` pair to automatically include doxygen comment
blocks as part of a group.
When a file contains multiple functions, classes, or structs in the same group you should instead
use the \@addtogroup with a `@{ ... @}` pair to automatically include doxygen comment blocks as
part of a group.

namespace cuspatial {
/**
Expand Down
82 changes: 0 additions & 82 deletions cpp/include/cuspatial/bounding_box.cuh

This file was deleted.

179 changes: 179 additions & 0 deletions cpp/include/cuspatial/bounding_boxes.cuh
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
/*
* Copyright (c) 2022, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#pragma once

#include <cuspatial/traits.hpp>

#include <rmm/cuda_stream_view.hpp>

namespace cuspatial {

/**
* @addtogroup spatial_relationship
* @{
*/

/**
* @brief Compute the spatial bounding boxes of sequences of points.
*
* Computes a bounding box around all points within each group (consecutive points with the same
* ID). This function can be applied to trajectory data, polygon vertices, linestring vertices, or
* any grouped point data.
*
* Before merging bounding boxes, each point may be expanded into a bounding box using an
* optional @p expansion_radius. The point is expanded to a box with coordinates
* `(point.x - expansion_radius, point.y - expansion_radius)` and
* `(point.x + expansion_radius, point.y + expansion_radius)`.
*
* @note Assumes Object IDs and points are presorted by ID.
*
* @tparam IdInputIt Iterator over object IDs. Must meet the requirements of
* [LegacyRandomAccessIterator][LinkLRAI] and be device-readable.
* @tparam PointInputIt Iterator over points. Must meet the requirements of
* [LegacyRandomAccessIterator][LinkLRAI] and be device-readable.
* @tparam BoundingBoxOutputIt Iterator over output bounding boxes. Each element is a tuple of two
* points representing corners of the axis-aligned bounding box. The type of the points is the same
* as the `value_type` of PointInputIt. Must meet the requirements of
* [LegacyRandomAccessIterator][LinkLRAI] and be device-writeable.
*
* @param ids_first beginning of the range of input object ids
* @param ids_last end of the range of input object ids
* @param points_first beginning of the range of input point (x,y) coordinates
* @param bounding_boxes_first beginning of the range of output bounding boxes, one per trajectory
* @param expansion_radius radius to add to each point when computing its bounding box.
* @param stream the CUDA stream on which to perform computations.
*
* @return An iterator to the end of the range of output bounding boxes.
*
* [LinkLRAI]: https://en.cppreference.com/w/cpp/named_req/RandomAccessIterator
* "LegacyRandomAccessIterator"
*/
template <typename IdInputIt,
typename PointInputIt,
typename BoundingBoxOutputIt,
typename T = iterator_vec_base_type<PointInputIt>>
BoundingBoxOutputIt point_bounding_boxes(IdInputIt ids_first,
IdInputIt ids_last,
PointInputIt points_first,
BoundingBoxOutputIt bounding_boxes_first,
T expansion_radius = T{0},
rmm::cuda_stream_view stream = rmm::cuda_stream_default);

/**
* @brief Compute minimum bounding box for each linestring.
*
* @tparam LinestringOffsetIterator Iterator type to linestring offsets. Must meet the requirements
* of [LegacyRandomAccessIterator][LinkLRAI] and be device-readable.
* @tparam VertexIterator Iterator type to linestring vertices. Must meet the requirements of
* [LegacyRandomAccessIterator][LinkLRAI] and be device-readable.
* @tparam BoundingBoxIterator Iterator type to bounding boxes. Must be writable using data of type
* `cuspatial::box<T>`. Must meet the requirements of [LegacyRandomAccessIterator][LinkLRAI] and be
* device-writeable.
* @tparam T The coordinate data value type.
* @tparam IndexT The offset data value type.
* @param linestring_offsets_first Iterator to beginning of the range of input polygon offsets.
* @param linestring_offsets_last Iterator to end of the range of input polygon offsets.
* @param linestring_vertices_first Iterator to beginning of the range of input polygon vertices.
* @param linestring_vertices_last Iterator to end of the range of input polygon vertices.
* @param bounding_boxes_first Iterator to beginning of the range of output bounding boxes.
* @param expansion_radius Optional radius to expand each vertex of the output bounding boxes.
* @param stream the CUDA stream on which to perform computations and allocate memory.
*
* @return An iterator to the end of the range of output bounding boxes.
*
* @pre For compatibility with GeoArrow, the number of linestring offsets
* `std::distance(linestring_offsets_first, linestring_offsets_last)` should be one more than the
* number of linestrings. The final offset is not used by this function, but the number of offsets
* determines the output size.
*
* [LinkLRAI]: https://en.cppreference.com/w/cpp/named_req/RandomAccessIterator
* "LegacyRandomAccessIterator"
*/
template <class LinestringOffsetIterator,
class VertexIterator,
class BoundingBoxIterator,
class T = iterator_vec_base_type<VertexIterator>,
class IndexT = iterator_value_type<LinestringOffsetIterator>>
BoundingBoxIterator linestring_bounding_boxes(
LinestringOffsetIterator linestring_offsets_first,
LinestringOffsetIterator linestring_offsets_last,
VertexIterator linestring_vertices_first,
VertexIterator linestring_vertices_last,
BoundingBoxIterator bounding_boxes_first,
T expansion_radius = T{0},
rmm::cuda_stream_view stream = rmm::cuda_stream_default);

/**
* @brief Compute minimum bounding box for each polygon.
*
* @tparam PolygonOffsetIterator Iterator type to polygon offsets. Must meet the requirements of
* [LegacyRandomAccessIterator][LinkLRAI] and be device-readable.
* @tparam RingOffsetIterator Iterator type to polygon ring offsets. Must meet the requirements of
* [LegacyRandomAccessIterator][LinkLRAI] and be device-readable.
* @tparam VertexIterator Iterator type to polygon vertices. Must meet the requirements of
* [LegacyRandomAccessIterator][LinkLRAI] and be device-readable.
* @tparam BoundingBoxIterator Iterator type to bounding boxes. Must be writable using data of type
* `cuspatial::box<T>`. Must meet the requirements of [LegacyRandomAccessIterator][LinkLRAI] and be
* device-writeable.
* @tparam T The coordinate data value type.
* @tparam IndexT The offset data value type.
* @param polygon_offsets_first Iterator to beginning of the range of input polygon offsets.
* @param polygon_offsets_last Iterator to end of the range of input polygon offsets.
* @param polygon_ring_offsets_first Iterator to beginning of the range of input polygon ring
* offsets.
* @param polygon_ring_offsets_last Iterator to end of the range of input polygon ring offsets.
* @param polygon_vertices_first Iterator to beginning of the range of input polygon vertices.
* @param polygon_vertices_last Iterator to end of the range of input polygon vertices.
* @param bounding_boxes_first Iterator to beginning of the range of output bounding boxes.
* @param expansion_radius Optional radius to expand each vertex of the output bounding boxes.
* @param stream the CUDA stream on which to perform computations and allocate memory.
*
* @return An iterator to the end of the range of output bounding boxes.
*
* @pre For compatibility with GeoArrow, the number of polygon offsets
* `std::distance(polygon_offsets_first, polygon_offsets_last)` should be one more than the number
* of polygons. The number of ring offsets `std::distance(polygon_ring_offsets_first,
* polygon_ring_offsets_last)` should be one more than the number of total rings. The
* final offset in each range is not used by this function, but the number of polygon offsets
* determines the output size.
*
* [LinkLRAI]: https://en.cppreference.com/w/cpp/named_req/RandomAccessIterator
* "LegacyRandomAccessIterator"
*/
template <class PolygonOffsetIterator,
class RingOffsetIterator,
class VertexIterator,
class BoundingBoxIterator,
class T = iterator_vec_base_type<VertexIterator>,
class IndexT = iterator_value_type<PolygonOffsetIterator>>
BoundingBoxIterator polygon_bounding_boxes(PolygonOffsetIterator polygon_offsets_first,
PolygonOffsetIterator polygon_offsets_last,
RingOffsetIterator polygon_ring_offsets_first,
RingOffsetIterator polygon_ring_offsets_last,
VertexIterator polygon_vertices_first,
VertexIterator polygon_vertices_last,
BoundingBoxIterator bounding_boxes_first,
T expansion_radius = T{0},
rmm::cuda_stream_view stream = rmm::cuda_stream_default);

/**
* @} // end of doxygen group
*/

} // namespace cuspatial

#include <cuspatial/detail/bounding_boxes.cuh>
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,38 @@
namespace cuspatial {

/**
* @brief Compute minimum bounding box for each polygon in a list.
* @addtogroup spatial_relationship
* @{
*/

/**
* @brief Compute minimum bounding boxes of a set of linestrings and an expansion radius.
*
* @param linestring_offsets Begin indices of the first point in each linestring (i.e. prefix-sum)
* @param x Linestring point x-coordinates
* @param y Linestring point y-coordinates
* @param expansion_radius Radius of each linestring point
*
* @return a cudf table of bounding boxes as four columns of the same type as `x` and `y`:
* x_min - the minimum x-coordinate of each bounding box
* y_min - the minimum y-coordinate of each bounding box
* x_max - the maximum x-coordinate of each bounding box
* y_max - the maximum y-coordinate of each bounding box
*
* @ingroup spatial_relationship
* @pre For compatibility with GeoArrow, the size of @p linestring_offsets should be one more than
* the number of linestrings to process. The final offset is not used by this function, but the
* number of offsets determines the output size.
*/

std::unique_ptr<cudf::table> linestring_bounding_boxes(
cudf::column_view const& linestring_offsets,
cudf::column_view const& x,
cudf::column_view const& y,
double expansion_radius,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());

/**
* @brief Compute minimum bounding box for each polygon in a list.
*
* @param poly_offsets Begin indices of the first ring in each polygon (i.e. prefix-sum)
* @param ring_offsets Begin indices of the first point in each ring (i.e. prefix-sum)
Expand All @@ -54,4 +83,8 @@ std::unique_ptr<cudf::table> polygon_bounding_boxes(
double expansion_radius = 0.0,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());

/**
* @} // end of doxygen group
*/

} // namespace cuspatial
3 changes: 2 additions & 1 deletion cpp/include/cuspatial/column/geometry_column_view.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@
namespace cuspatial {

/**
* @ingroup cuspatial_types
* @brief A non-owning, immutable view of a geometry column.
*
* @ingroup cuspatial_types
*
* A geometry column is GeoArrow compliant, except that the data type for
* the coordinates is List<T>, instead of FixedSizeList<T>[n_dim]. This is
* because libcudf does not support FixedSizeList type. Currently, an
Expand Down
Loading

0 comments on commit 2509fce

Please sign in to comment.