Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Comprehensive Test for Multigeometry Range Classes #1197

Merged
merged 22 commits into from
Jul 31, 2023
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cpp/include/cuspatial/detail/geometry/polygon_ref.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain these changes? Why can't point_begin be used as stored?

Copy link
Contributor Author

@isVoid isVoid Jul 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tl;dr. Because point_begin API is supposed to return only the point range to the current multilinestring (not all multilinestrings). And that the offsets stored in a *_ref are global offsets to the lower level range.

Consider this example:

Multilinestring_range:
Geometry offsets: [0, 2, 4]
Part Offsets: [0, 2, 4, 6, 8]
Points: [P0, P1... P8]

A multilinestring_ref constructed for the second multilinestring is stored as below:

Part Offsets: [4, 6, 8]
Points: [P0, ... P8]

Notice that in part offset, the offsets are global offsets (4 means that the first point to the multilinestring locates at _point_begin + 4). You may ask why we don't store as below:

Part Offsets: [0, 2, 4]
Points: [P4 ... P8]

That's an alternative too. Just different ways to skin a cat. I don't see an advantage to either one.

}

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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 is the one-past the last part index to the point of this multilinestring.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// _part_end is the one-past the last part index to the point of this multilinestring.
// _part_end refers to one past the last part index to the points of this multilinestring.

I don't quite understand this. A part index indexes points? Is it one past the last point of the part?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See how multilinestring_ref is constructed:

https://github.com/isVoid/cuspatial/blob/293e4c30b5b43ea57225fe4e52a55e18f4e99e3a/cpp/include/cuspatial/detail/range/multilinestring_range.cuh#L67

*(_part_begin + _geometry_begin[i + 1]) is the part index of the last point of the current multilinestring. When constructing it, I increment it to maintain that the the end interator of a range should be one-past the last element.

// 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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
41 changes: 0 additions & 41 deletions cpp/include/cuspatial/detail/range/multipolygon_range.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions cpp/include/cuspatial/detail/utility/zero_data.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

#include <cuspatial/traits.hpp>

#include <rmm/cuda_stream_view.hpp>

isVoid marked this conversation as resolved.
Show resolved Hide resolved
namespace cuspatial {
namespace detail {

Expand Down
8 changes: 8 additions & 0 deletions cpp/include/cuspatial/iterator_factory.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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
*/
Expand Down
16 changes: 7 additions & 9 deletions cpp/include/cuspatial/range/multilinestring_range.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down Expand Up @@ -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();
Expand All @@ -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.
Expand Down
1 change: 1 addition & 0 deletions cpp/include/cuspatial/range/multipoint_range.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
23 changes: 2 additions & 21 deletions cpp/include/cuspatial/range/multipolygon_range.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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; }
Expand All @@ -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);
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion cpp/include/cuspatial_test/geometry_generator.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down
69 changes: 52 additions & 17 deletions cpp/include/cuspatial_test/vector_factories.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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))
{
}

Expand Down Expand Up @@ -271,22 +282,34 @@ class multilinestring_array {
* @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 ranges
isVoid marked this conversation as resolved.
Show resolved Hide resolved
*
* @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,
isVoid marked this conversation as resolved.
Show resolved Hide resolved
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));
}

/**
Expand Down Expand Up @@ -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
1 change: 1 addition & 0 deletions cpp/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Loading