Add C++ API for `point_linestring_nearest_points` #658

isVoid · 2022-08-20T00:08:49Z

This PR adds C++ API of point_linestring_nearest_points. The host counterpart of this function is shapely.ops.nearest_points. Multiple fields are written when called on this API, including the geometry and part id indicating the geometry that the nearest point is on. In the header only API, the output iterator accepts a zipped iterator made of 4 output iterators. In the column API, a tuple is returned.

The kernel is parallelized per geometry. Each thread computes the nearest point between a pair of multipoint and multilinestring.

Contribute to #646

…_linestring_distance

…string_distance

This reverts commit a8ec5ff.

This reverts commit 2ed60eb.

…into feature/header_only_point_linestring_distance

Co-authored-by: Mark Harris <mharris@nvidia.com>

…b.com:isVoid/cuspatial into feature/header_only_point_linestring_distance

…into feature/header_only_point_linestring_distance

…into feature/pairwise_point_to_nearest_linestring_point

isVoid · 2022-09-19T18:59:15Z

Hi, I noticed that cython does not yet support std::tuple due to lack of variadic template support. Therefore pairwise_point_linestring_nearest_points cannot return a tuple of the results. We'll need to construct a custom struct for the return type. Working on a change.

isVoid · 2022-09-19T19:32:05Z

Hi, I noticed that cython does not yet support std::tuple due to lack of variadic template support. Therefore pairwise_point_linestring_nearest_points cannot return a tuple of the results. We'll need to construct a custom struct for the return type. Working on a change.

done.

isVoid · 2022-09-19T22:01:29Z

rerun tests

harrism

Thanks @isVoid !

cpp/include/cuspatial/experimental/detail/point_linestring_nearest_points.cuh

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

harrism · 2022-09-20T02:02:01Z

cpp/include/cuspatial/experimental/detail/point_linestring_nearest_points.cuh

+    thrust::distance(points_geometry_offsets_first, points_geometry_offsets_last) - 1;
+  auto num_linestring_points = thrust::distance(linestring_points_first, linestring_points_last);
+
+  for (auto idx = threadIdx.x + blockIdx.x * blockDim.x; idx < num_pairs;


Seeing this quadruple nested loop, I see now how important it will be to add automatic geometry iterators that know how to do the nested indexing. You want to be able to just iterate over the pairs, and for each pair, just have a for_each over the points and a nested for_each over the points of the multilinestring.

Yes, abstracting different level of iteration is direly needed. Perhaps we can continue the discussion in #677 ?

cpp/include/cuspatial/experimental/point_linestring_nearest_points.cuh

cpp/include/cuspatial/point_linestring_nearest_points.hpp

harrism · 2022-09-20T02:30:15Z

cpp/include/cuspatial/point_linestring_nearest_points.hpp

+ * ```
+ *
+ * @param multipoint_geometry_offsets Beginning and ending indices for each multipoint
+ * @param points_xy Interleaved x, y-coordinate of points


interleaved x and y doesn't match other cuspatial column-based APIs. We have always had the X/Y and Lon/Lat positions in separate columns, right?

We started making this adaptation for #649. We could keep them separate but on the python side this would always require 2 scatter operations before we can call c++.

Oh. I was under the impression that if you are using the column-based API you are using dataframes, which tend to have a separate column for each field. What does GeoPandas do here?

A GeoSeries in geopandas is an array of structures. There's currently a discussion over the format of xy-coordinates going on at geoarrow:
geoarrow/geoarrow#26

We argued successfully to use xy interleaved coordinates for points to improve cache coherency and word size (coalesced memory access) availability for kernels when GeoArrow was designed. @isVoid has been in the process of converting our existing APIs to support GeoArrow more directly. I'll be following up in the above conversation today.

cpp/tests/utility/vector_equality.hpp

cpp/include/cuspatial/experimental/point_linestring_nearest_points.cuh

cpp/include/cuspatial/point_linestring_nearest_points.hpp

thomcom · 2022-09-20T20:17:33Z

cpp/include/cuspatial/point_linestring_nearest_points.hpp

+ * ```
+ *
+ * @param multipoint_geometry_offsets Beginning and ending indices for each multipoint
+ * @param points_xy Interleaved x, y-coordinate of points


We argued successfully to use xy interleaved coordinates for points to improve cache coherency and word size (coalesced memory access) availability for kernels when GeoArrow was designed. @isVoid has been in the process of converting our existing APIs to support GeoArrow more directly. I'll be following up in the above conversation today.

harrism · 2022-09-21T01:29:28Z

rerun tests

isVoid · 2022-09-21T23:39:15Z

Upon addressing this comment: #658 (comment), I introduced a bug overwrites nearest_points under every iteration. When I attempt to fix this bug, I realized that thrust::tuple still doesn't work well with structure binding. (The binding works, but the bounded variable is a thrust::detail::cons) that requires more unwrapping. I simply reverted this change.

for thrust::tuple min repro: https://godbolt.org/z/drTr4dx8z

Co-authored-by: H. Thomson Comer <thomcom@gmail.com>

thomcom · 2022-09-22T17:11:41Z

I think this is ready to merge @isVoid? Congrats! :)

isVoid · 2022-09-22T17:58:57Z

@gpucibot merge

This PR adds python API for point-linestring nearest points. Closes #646 . Depend on #658 #686 This PR also moves `Feature_Enum` from io module to geometa module to make sure the coding is consistent and reused throughout the codebase. Authors: - Michael Wang (https://github.com/isVoid) Approvers: - Mark Harris (https://github.com/harrism) - H. Thomson Comer (https://github.com/thomcom) URL: #681

isVoid and others added 30 commits June 14, 2022 10:52

Initial to atomics refactoring

de78310

Initial refactoring geometry utilities

6011900

Add header only api

9761eab

Add cudf column API

2ed60eb

cmake updates

e3ec12d

inline atomics to avoid compiling RDC

a72ac10

Rename to proper naming convention

31b1e78

make all atomics inline

f55af2f

Merge branch 'improvement/device_atomics_refactor' into feature/point…

e874f3b

…_linestring_distance

Merge branch 'improvement/geometry_utilities' into feature/point_line…

e9b2ca8

…string_distance

Changes header only API to accept only single offset iterator

f2277f0

Add header only API tests

7bc0914

Update cudf column API calls

a8ec5ff

Initial add utility method

1fdfd59

Add counting transform iterators

0274875

Revert "Update cudf column API calls"

df286a0

This reverts commit a8ec5ff.

Revert "Add cudf column API"

688fbb5

This reverts commit 2ed60eb.

Merge branch 'branch-22.08' of https://github.com/rapidsai/cuspatial …

d0c0c4f

…into feature/header_only_point_linestring_distance

Update refactored helper locations

1c4ad61

Add cudf column API

e4038a5

Add equality test utility and fix header only tests

5004bbe

minor docstring updates

986dbb2

Add cudf column API documentation

2feab4b

Include point-linestring distance files in docs.

5e07194

Apply suggestions from code review

4834aa2

Address review comments

de0feb7

Co-authored-by: Mark Harris <mharris@nvidia.com>

Merge branch 'feature/header_only_point_linestring_distance' of githu…

c897974

…b.com:isVoid/cuspatial into feature/header_only_point_linestring_distance

Merge branch 'branch-22.10' of https://github.com/rapidsai/cuspatial …

6dc34e0

…into feature/header_only_point_linestring_distance

Fix broken builds

624a97f

Initial scaffolding for nearest point

7d8b4b6

isVoid requested review from harrism and thomcom and removed request for zhangjianting September 14, 2022 01:03

isVoid added 3 commits September 14, 2022 17:41

Merge branch 'branch-22.10' of https://github.com/rapidsai/cuspatial …

15af211

…into feature/pairwise_point_to_nearest_linestring_point

use latest vector<vec_2d> matcher

6c13809

Merge branch 'branch-22.10' of https://github.com/rapidsai/cuspatial …

946bd10

…into feature/pairwise_point_to_nearest_linestring_point

replace tuple with custom struct

cb80449

isVoid added 2 commits September 19, 2022 13:20

rename column API file and add API to doc page

1c8ea50

fix tests after rename

fceafaf

isVoid mentioned this pull request Sep 20, 2022

Python API for point-linestring nearest points #681

Merged

3 tasks

harrism requested changes Sep 20, 2022

View reviewed changes

address review comments in bulk

ce1d8b7

isVoid added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currenty a work in progress labels Sep 20, 2022

thomcom approved these changes Sep 20, 2022

View reviewed changes

harrism approved these changes Sep 21, 2022

View reviewed changes

isVoid added 3 commits September 21, 2022 16:22

revert the use of thrust::tuple

9dd1a1e

Add two tests for nearest end points

f72ef3d

add more corner case tests

3fba1c4

isVoid and others added 2 commits September 21, 2022 16:45

Apply suggestions from code review

ed1bf28

Co-authored-by: H. Thomson Comer <thomcom@gmail.com>

style

8f5d7c1

rapids-bot bot merged commit e9a2faa into rapidsai:branch-22.10 Sep 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add C++ API for `point_linestring_nearest_points` #658

Add C++ API for `point_linestring_nearest_points` #658

isVoid commented Aug 20, 2022 •

edited

Loading

isVoid commented Sep 19, 2022

isVoid commented Sep 19, 2022

isVoid commented Sep 19, 2022

harrism left a comment

harrism Sep 20, 2022

isVoid Sep 20, 2022

harrism Sep 20, 2022

isVoid Sep 20, 2022 •

edited

Loading

harrism Sep 20, 2022

isVoid Sep 20, 2022 •

edited

Loading

thomcom Sep 20, 2022

thomcom Sep 20, 2022

harrism commented Sep 21, 2022

isVoid commented Sep 21, 2022

thomcom commented Sep 22, 2022

isVoid commented Sep 22, 2022

Add C++ API for point_linestring_nearest_points #658

Add C++ API for point_linestring_nearest_points #658

Conversation

isVoid commented Aug 20, 2022 • edited Loading

isVoid commented Sep 19, 2022

isVoid commented Sep 19, 2022

isVoid commented Sep 19, 2022

harrism left a comment

Choose a reason for hiding this comment

harrism Sep 20, 2022

Choose a reason for hiding this comment

isVoid Sep 20, 2022

Choose a reason for hiding this comment

harrism Sep 20, 2022

Choose a reason for hiding this comment

isVoid Sep 20, 2022 • edited Loading

Choose a reason for hiding this comment

harrism Sep 20, 2022

Choose a reason for hiding this comment

isVoid Sep 20, 2022 • edited Loading

Choose a reason for hiding this comment

thomcom Sep 20, 2022

Choose a reason for hiding this comment

thomcom Sep 20, 2022

Choose a reason for hiding this comment

harrism commented Sep 21, 2022

isVoid commented Sep 21, 2022

thomcom commented Sep 22, 2022

isVoid commented Sep 22, 2022

Add C++ API for `point_linestring_nearest_points` #658

Add C++ API for `point_linestring_nearest_points` #658

isVoid commented Aug 20, 2022 •

edited

Loading

isVoid Sep 20, 2022 •

edited

Loading

isVoid Sep 20, 2022 •

edited

Loading