Add Comprehensive Test for Multigeometry Range Classes (#1197)

closes #991

This PR adds comprehensive tests for `multi*_range` class. The basic goal is to make sure every API in service is at least covered by 3 tests: a range with 0, 1 and 1000 elements. The tests are structured as below: for each range, a base fixture class is created. The base class defines virtual functions that its subclass should implement. These virtual functions include a `make_test_multi*` function that constructs the geometry array of that specific test case, as well as each sub test function that defines the expected value of that test case. The base class defines a `SetUp` function that calls the `make_test_multi*` to generate the array at test start up time. Then, a `run_test` function is defined to subsequently call every sub routine that test every API of that geometry range.

In addition, this PR fixes several small bug in `point_begin` accessor in `linestring_ref` and `multipolygon_ref` class. Also, a few unused functions in `multipolygon_range` have been removed.

Authors:
  - Michael Wang (https://github.com/isVoid)

Approvers:
  - Robert Maynard (https://github.com/robertmaynard)
  - Mark Harris (https://github.com/harrism)
  - H. Thomson Comer (https://github.com/thomcom)

URL: #1197

cpp/include/cuspatial/detail/geometry/polygon_ref.cuh

@@ -59,13 +59,13 @@ CUSPATIAL_HOST_DEVICE auto polygon_ref<RingIterator, VecIterator>::ring_end() co
 template <typename RingIterator, typename VecIterator>
 CUSPATIAL_HOST_DEVICE auto polygon_ref<RingIterator, VecIterator>::point_begin() const
 {
-  return _point_begin;
+  return thrust::next(_point_begin, *_ring_begin);
 }
 
 template <typename RingIterator, typename VecIterator>
 CUSPATIAL_HOST_DEVICE auto polygon_ref<RingIterator, VecIterator>::point_end() const
 {
-  return _point_end;
+  return thrust::next(_point_begin, *thrust::prev(_ring_end));
 }
 
 template <typename RingIterator, typename VecIterator>

cpp/include/cuspatial/detail/geometry_collection/multilinestring_ref.cuh

@@ -74,13 +74,15 @@ CUSPATIAL_HOST_DEVICE auto multilinestring_ref<PartIterator, VecIterator>::part_
 template <typename PartIterator, typename VecIterator>
 CUSPATIAL_HOST_DEVICE auto multilinestring_ref<PartIterator, VecIterator>::point_begin() const
 {
-  return _point_begin;
+  return thrust::next(_point_begin, *_part_begin);
 }
 
 template <typename PartIterator, typename VecIterator>
 CUSPATIAL_HOST_DEVICE auto multilinestring_ref<PartIterator, VecIterator>::point_end() const
 {
-  return _point_end;
+  // _part_end refers to the one past the last part index to the points of this multilinestring.
+  // So prior to computing the end point index, we need to decrement _part_end.
+  return thrust::next(_point_begin, *thrust::prev(_part_end));
 }
 
 template <typename PartIterator, typename VecIterator>

cpp/include/cuspatial/detail/geometry_collection/multipolygon_ref.cuh

@@ -112,14 +112,14 @@ template <typename PartIterator, typename RingIterator, typename VecIterator>
 CUSPATIAL_HOST_DEVICE auto multipolygon_ref<PartIterator, RingIterator, VecIterator>::point_begin()
   const
 {
-  return _point_begin;
+  return thrust::next(_point_begin, *thrust::next(_ring_begin, *_part_begin));
 }
 
 template <typename PartIterator, typename RingIterator, typename VecIterator>
 CUSPATIAL_HOST_DEVICE auto multipolygon_ref<PartIterator, RingIterator, VecIterator>::point_end()
   const
 {
-  return _point_end;
+  return thrust::next(_point_begin, *thrust::next(_ring_begin, *thrust::prev(_part_end)));
 }
 
 template <typename PartIterator, typename RingIterator, typename VecIterator>

cpp/include/cuspatial/detail/range/multilinestring_range.cuh

@@ -200,12 +200,11 @@ multilinestring_range<GeometryIterator, PartIterator, VecIterator>::is_valid_seg
 
 template <typename GeometryIterator, typename PartIterator, typename VecIterator>
 template <typename IndexType>
-CUSPATIAL_HOST_DEVICE thrust::pair<
-  vec_2d<typename multilinestring_range<GeometryIterator, PartIterator, VecIterator>::element_t>,
-  vec_2d<typename multilinestring_range<GeometryIterator, PartIterator, VecIterator>::element_t>>
+CUSPATIAL_HOST_DEVICE auto
 multilinestring_range<GeometryIterator, PartIterator, VecIterator>::segment(IndexType segment_idx)
 {
-  return thrust::make_pair(_point_begin[segment_idx], _point_begin[segment_idx + 1]);
+  using T = iterator_vec_base_type<VecIterator>;
+  return cuspatial::segment<T>{_point_begin[segment_idx], _point_begin[segment_idx + 1]};
 }
 
 template <typename GeometryIterator, typename PartIterator, typename VecIterator>

cpp/include/cuspatial/detail/range/multilinestring_segment_range.cuh

@@ -200,7 +200,7 @@ class multilinestring_segment_range {
   CUSPATIAL_HOST_DEVICE auto multigeometry_offset_begin()
   {
     return thrust::make_permutation_iterator(_per_linestring_offset_begin(),
-                                             _parent.geometry_offsets_begin());
+                                             _parent.geometry_offset_begin());
   }
 
   /// Returns end iterator to the range of the starting segment index per multilinestring

cpp/include/cuspatial/detail/range/multipolygon_range.cuh

@@ -241,23 +241,6 @@ multipolygon_range<GeometryIterator, PartIterator, RingIterator, VecIterator>::
     thrust::prev(thrust::upper_bound(thrust::seq, _geometry_begin, _geometry_end, part_idx)));
 }
 
-template <typename GeometryIterator,
-          typename PartIterator,
-          typename RingIterator,
-          typename VecIterator>
-template <typename IndexType>
-CUSPATIAL_HOST_DEVICE auto
-multipolygon_range<GeometryIterator, PartIterator, RingIterator, VecIterator>::
-  geometry_idx_from_segment_idx(IndexType segment_idx)
-{
-  auto ring_idx = ring_idx_from_point_idx(segment_idx);
-  if (!is_valid_segment_id(segment_idx, ring_idx))
-    return multipolygon_range<GeometryIterator, PartIterator, RingIterator, VecIterator>::
-      INVALID_INDEX;
-
-  return geometry_idx_from_part_idx(part_idx_from_ring_idx(ring_idx));
-}
-
 template <typename GeometryIterator,
           typename PartIterator,
           typename RingIterator,
@@ -343,18 +326,6 @@ multipolygon_range<GeometryIterator, PartIterator, RingIterator, VecIterator>::o
   return multipolygon_begin()[multipolygon_idx];
 }
 
-template <typename GeometryIterator,
-          typename PartIterator,
-          typename RingIterator,
-          typename VecIterator>
-template <typename IndexType>
-CUSPATIAL_HOST_DEVICE auto
-multipolygon_range<GeometryIterator, PartIterator, RingIterator, VecIterator>::get_segment(
-  IndexType segment_idx)
-{
-  return segment{_point_begin[segment_idx], _point_begin[segment_idx + 1]};
-}
-
 template <typename GeometryIterator,
           typename PartIterator,
           typename RingIterator,
@@ -366,18 +337,6 @@ auto multipolygon_range<GeometryIterator, PartIterator, RingIterator, VecIterato
   return multilinestring_segment_manager{multilinestring_range, stream};
 }
 
-template <typename GeometryIterator,
-          typename PartIterator,
-          typename RingIterator,
-          typename VecIterator>
-template <typename IndexType1, typename IndexType2>
-CUSPATIAL_HOST_DEVICE bool
-multipolygon_range<GeometryIterator, PartIterator, RingIterator, VecIterator>::
-  is_first_point_of_multipolygon(IndexType1 point_idx, IndexType2 geometry_idx)
-{
-  return point_idx == _ring_begin[_part_begin[_geometry_begin[geometry_idx]]];
-}
-
 template <typename GeometryIterator,
           typename PartIterator,
           typename RingIterator,

cpp/include/cuspatial/detail/utility/zero_data.cuh

@@ -19,6 +19,8 @@
 
 #include <cuspatial/traits.hpp>
 
+#include <rmm/cuda_stream_view.hpp>
+
 namespace cuspatial {
 namespace detail {
 

cpp/include/cuspatial/iterator_factory.cuh

@@ -16,6 +16,7 @@
 
 #pragma once
 
+#include <cuspatial/detail/functors.cuh>
 #include <cuspatial/error.hpp>
 #include <cuspatial/geometry/box.hpp>
 #include <cuspatial/geometry/vec_2d.hpp>
@@ -424,6 +425,13 @@ auto make_geometry_id_iterator(GeometryIter geometry_offsets_begin,
                  std::distance(geometry_offsets_begin, geometry_offsets_end)));
 }
 
+template <typename OffsetIterator>
+auto make_count_iterator_from_offset_iterator(OffsetIterator it)
+{
+  auto zipped_offsets_it = thrust::make_zip_iterator(it, thrust::next(it));
+  return thrust::make_transform_iterator(zipped_offsets_it, detail::offset_pair_to_count_functor{});
+}
+
 /**
  * @} // end of doxygen group
  */

cpp/include/cuspatial/range/multilinestring_range.cuh

@@ -101,6 +101,12 @@ class multilinestring_range {
   /// Return the iterator to the one past the last point in the range.
   CUSPATIAL_HOST_DEVICE auto point_end() { return _point_end; }
 
+  /// Return the iterator to the first geometry offset in the range.
+  CUSPATIAL_HOST_DEVICE auto geometry_offset_begin() { return _geometry_begin; }
+
+  /// Return the iterator to the one past the last geometry offset in the range.
+  CUSPATIAL_HOST_DEVICE auto geometry_offset_end() { return _geometry_end; }
+
   /// Return the iterator to the first part offset in the range.
   CUSPATIAL_HOST_DEVICE auto part_offset_begin() { return _part_begin; }
 
@@ -144,8 +150,7 @@ class multilinestring_range {
 
   /// Returns the segment given a segment index.
   template <typename IndexType>
-  CUSPATIAL_HOST_DEVICE thrust::pair<vec_2d<element_t>, vec_2d<element_t>> segment(
-    IndexType segment_idx);
+  CUSPATIAL_HOST_DEVICE auto segment(IndexType segment_idx);
 
   /// Returns an iterator to the counts of points per multilinestring
   CUSPATIAL_HOST_DEVICE auto multilinestring_point_count_begin();
@@ -168,13 +173,6 @@ class multilinestring_range {
   template <typename IndexType>
   CUSPATIAL_HOST_DEVICE auto operator[](IndexType multilinestring_idx);
 
-  /// Raw offsets iterator
-
-  CUSPATIAL_HOST_DEVICE auto geometry_offsets_begin() { return _geometry_begin; }
-  CUSPATIAL_HOST_DEVICE auto geometry_offsets_end() { return _geometry_end; }
-  CUSPATIAL_HOST_DEVICE auto part_offsets_begin() { return _part_begin; }
-  CUSPATIAL_HOST_DEVICE auto part_offsets_end() { return _part_end; }
-
   /// Range Casts
 
   /// Casts the multilinestring range into a multipoint range.

cpp/include/cuspatial/range/multipoint_range.cuh

@@ -149,6 +149,7 @@ class multipoint_range {
 
   /**
    * @brief Returns `true` if the range contains only single points
+   * Undefined behavior if the range is an empty range.
    */
   CUSPATIAL_HOST_DEVICE bool is_single_point_range();
 

cpp/include/cuspatial/range/multipolygon_range.cuh

@@ -72,8 +72,6 @@ class multipolygon_range {
   using index_t   = iterator_value_type<GeometryIterator>;
   using element_t = iterator_vec_base_type<VecIterator>;
 
-  int64_t static constexpr INVALID_INDEX = -1;
-
   multipolygon_range(GeometryIterator geometry_begin,
                      GeometryIterator geometry_end,
                      PartIterator part_begin,
@@ -117,10 +115,10 @@ class multipolygon_range {
   CUSPATIAL_HOST_DEVICE auto point_end();
 
   /// Return the iterator to the first geometry offset in the range.
-  CUSPATIAL_HOST_DEVICE auto geometry_offsets_begin() { return _part_begin; }
+  CUSPATIAL_HOST_DEVICE auto geometry_offset_begin() { return _part_begin; }
 
   /// Return the iterator to the one past the last geometry offset in the range.
-  CUSPATIAL_HOST_DEVICE auto geometry_offsets_end() { return _part_end; }
+  CUSPATIAL_HOST_DEVICE auto geometry_offset_end() { return _part_end; }
 
   /// Return the iterator to the first part offset in the range.
   CUSPATIAL_HOST_DEVICE auto part_offset_begin() { return _part_begin; }
@@ -134,13 +132,6 @@ class multipolygon_range {
   /// Return the iterator to the one past the last ring offset in the range.
   CUSPATIAL_HOST_DEVICE auto ring_offset_end() { return _ring_end; }
 
-  /// Given the index of a segment, return the index of the geometry (multipolygon) that contains
-  /// the segment. Segment index is the index to the starting point of the segment. If the index is
-  /// the last point of the ring, then it is not a valid index. This function returns
-  /// multipolygon_range::INVALID_INDEX if the index is invalid.
-  template <typename IndexType>
-  CUSPATIAL_HOST_DEVICE auto geometry_idx_from_segment_idx(IndexType segment_idx);
-
   /// Given the index of a point, return the index of the ring that contains the point.
   template <typename IndexType>
   CUSPATIAL_HOST_DEVICE auto ring_idx_from_point_idx(IndexType point_idx);
@@ -158,16 +149,6 @@ class multipolygon_range {
   template <typename IndexType>
   CUSPATIAL_HOST_DEVICE auto operator[](IndexType multipolygon_idx);
 
-  /// Returns the `segment_idx`th segment in the multipolygon range.
-  template <typename IndexType>
-  CUSPATIAL_HOST_DEVICE auto get_segment(IndexType segment_idx);
-
-  /// Returns `true` if `point_idx`th point is the first point of `geometry_idx`th
-  /// multipolygon
-  template <typename IndexType1, typename IndexType2>
-  CUSPATIAL_HOST_DEVICE bool is_first_point_of_multipolygon(IndexType1 point_idx,
-                                                            IndexType2 geometry_idx);
-
   /// Returns an iterator to the number of points of the first multipolygon
   /// @note The count includes the duplicate first and last point of the ring.
   CUSPATIAL_HOST_DEVICE auto multipolygon_point_count_begin();

cpp/include/cuspatial_test/geometry_generator.cuh

@@ -354,7 +354,7 @@ auto generate_multilinestring_array(multilinestring_generator_parameter<T> param
                          params.origin,
                          detail::random_walk_functor<T>{params.segment_length});
 
-  return make_multilinestring_array(
+  return make_multilinestring_array<std::size_t, T>(
     std::move(geometry_offset), std::move(part_offset), std::move(points));
 }
 

cpp/include/cuspatial_test/vector_factories.cuh

@@ -101,6 +101,17 @@ class multipolygon_array {
   {
   }
 
+  multipolygon_array(rmm::device_vector<geometry_t>&& geometry_offsets_array,
+                     rmm::device_vector<part_t>&& part_offsets_array,
+                     rmm::device_vector<ring_t>&& ring_offsets_array,
+                     rmm::device_vector<coord_t>&& coordinates_array)
+    : _geometry_offsets_array(std::move(geometry_offsets_array)),
+      _part_offsets_array(std::move(part_offsets_array)),
+      _ring_offsets_array(std::move(ring_offsets_array)),
+      _coordinates_array(std::move(coordinates_array))
+  {
+  }
+
   multipolygon_array(rmm::device_uvector<geometry_t>&& geometry_offsets_array,
                      rmm::device_uvector<part_t>&& part_offsets_array,
                      rmm::device_uvector<ring_t>&& ring_offsets_array,
@@ -230,9 +241,9 @@ class multilinestring_array {
   multilinestring_array(GeometryArray geometry_offsets_array,
                         PartArray part_offsets_array,
                         CoordinateArray coordinate_array)
-    : _geometry_offset_array(geometry_offsets_array),
-      _part_offset_array(part_offsets_array),
-      _coordinate_array(coordinate_array)
+    : _geometry_offset_array(std::move(geometry_offsets_array)),
+      _part_offset_array(std::move(part_offsets_array)),
+      _coordinate_array(std::move(coordinate_array))
   {
   }
 
@@ -264,29 +275,41 @@ class multilinestring_array {
 };
 
 /**
- * @brief Construct an owning object of a multilinestring array from ranges
+ * @brief Construct an owning object of a multilinestring array from `device_uvectors`
  *
  * @param geometry_inl Range of geometry offsets
  * @param part_inl Range of part offsets
  * @param coord_inl Ramge of coordinate
  * @return multilinestring array object
  */
-template <typename IndexRangeA,
-          typename IndexRangeB,
-          typename CoordRange,
-          typename IndexType = typename IndexRangeB::value_type>
-auto make_multilinestring_array(IndexRangeA geometry_inl,
-                                IndexRangeB part_inl,
-                                CoordRange coord_inl)
+template <typename IndexType, typename T>
+auto make_multilinestring_array(rmm::device_uvector<IndexType>&& geometry_inl,
+                                rmm::device_uvector<IndexType>&& part_inl,
+                                rmm::device_uvector<vec_2d<T>>&& coord_inl)
 {
-  using CoordType         = typename CoordRange::value_type;
-  using DeviceIndexVector = thrust::device_vector<IndexType>;
-  using DeviceCoordVector = thrust::device_vector<CoordType>;
+  return multilinestring_array<rmm::device_uvector<IndexType>,
+                               rmm::device_uvector<IndexType>,
+                               rmm::device_uvector<vec_2d<T>>>(
+    std::move(geometry_inl), std::move(part_inl), std::move(coord_inl));
+}
 
-  return multilinestring_array<DeviceIndexVector, DeviceIndexVector, DeviceCoordVector>(
-    make_device_vector(std::move(geometry_inl)),
-    make_device_vector(std::move(part_inl)),
-    make_device_vector(std::move(coord_inl)));
+/**
+ * @brief Construct an owning object of a multilinestring array from `device_vectors`
+ *
+ * @param geometry_inl Range of geometry offsets
+ * @param part_inl Range of part offsets
+ * @param coord_inl Ramge of coordinate
+ * @return multilinestring array object
+ */
+template <typename IndexType, typename T>
+auto make_multilinestring_array(rmm::device_vector<IndexType>&& geometry_inl,
+                                rmm::device_vector<IndexType>&& part_inl,
+                                rmm::device_vector<vec_2d<T>>&& coord_inl)
+{
+  return multilinestring_array<rmm::device_vector<IndexType>,
+                               rmm::device_vector<IndexType>,
+                               rmm::device_vector<vec_2d<T>>>(
+    std::move(geometry_inl), std::move(part_inl), std::move(coord_inl));
 }
 
 /**
@@ -414,5 +437,17 @@ auto make_multipoint_array(rmm::device_uvector<IndexType> geometry_offsets,
     std::move(geometry_offsets), std::move(coords)};
 }
 
+/**
+ * @brief Factory method to construct multipoint array by moving the offsets and coordinates from
+ * `rmm::device_vector`.
+ */
+template <typename IndexType, typename T>
+auto make_multipoint_array(rmm::device_vector<IndexType> geometry_offsets,
+                           rmm::device_vector<vec_2d<T>> coords)
+{
+  return multipoint_array<rmm::device_vector<std::size_t>, rmm::device_vector<vec_2d<T>>>{
+    std::move(geometry_offsets), std::move(coords)};
+}
+
 }  // namespace test
 }  // namespace cuspatial

cpp/tests/CMakeLists.txt

@@ -171,6 +171,7 @@ ConfigureTest(SINUSOIDAL_PROJECTION_TEST_EXP
 
 # range
 ConfigureTest(RANGE_TEST_EXP
+    range/multipoint_range_test.cu
     range/multilinestring_range_test.cu
     range/multipolygon_range_test.cu)