Create `pairwise_point_in_polygon` to be used by pairwise `GeoSeries` #750

thomcom · 2022-10-24T17:39:22Z

Description

This PR provides two primary outcomes: it adds a pairwise_point_in_polygon API in the header-only and cudf-backed C++ functionality, and it modifies the is_point_in_polygon kernel to perform boundary exclusion.

The pairwise_point_in_polygon API has been added because GeoPandas.GeoSeries.contains only supports a pairwise operation and a many-to-one operation, but does not implement the all-pairs approach to point-in-polygon we developed.

Boundary exclusion has been added for two reasons: The main reason is because GeoPandas.GeoSeries.contains applies boundary exclusion: points that are in the boundary of a polygon are not considered to be "in" the polygon. The other reason to add boundary exclusion is because our results were previously undefined. Some boundary points were excluded in point-in-polygon, and others were not. This PR resolves that inconsistency.

Checklist

I am familiar with the Contributing Guidelines.
New or existing tests cover these changes.
The documentation is up to date with these changes.

isVoid

I think we should add a long input test to have more point/polygon points than the size of a grid.

cpp/include/cuspatial/experimental/pairwise_point_in_polygon.cuh

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

cpp/include/cuspatial/pairwise_point_in_polygon.hpp

cpp/tests/experimental/spatial/pairwise_point_in_polygon_test.cu

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

harrism

Need an epsilon when comparing to zero...

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

harrism

Other than the experimental epsilon code with printfs and my question about the utility of pairwise PiP for big data, looks good.

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

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

isVoid

Last few comments

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

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

harrism · 2022-11-07T22:04:06Z

rerun tests

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

cpp/include/cuspatial/pairwise_point_in_polygon.hpp

cpp/include/cuspatial/experimental/pairwise_point_in_polygon.cuh

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

cpp/tests/experimental/spatial/pairwise_point_in_polygon_test.cu

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

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

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

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

cpp/tests/experimental/spatial/pairwise_point_in_polygon_test.cu

isVoid

See comments above, adopt if you see fit.

Co-authored-by: Michael Wang <isVoid@users.noreply.github.com>

…m/cuspatial into feature/pairwise_point_in_polygon

harrism · 2022-11-29T21:41:32Z

@gpucibot merge

…749) This PR closes the above named issues relating to creating a .contains method and, more importantly, resolving boundary case inconsistency with `point_in_polygon`. ~As it stands the colinearity test I've added to `is_point_in_polygon` doubles the runtime of brute-force `point_in_polygon` and has no visible effect on the runtime of `quadtree_point_in_polygon`.~ ~- Note I need to double check the above benchmark, having set this project down for the last few weeks.~ This depends on #750, please do not review the C++ code here until that PR is merged. Please do review the python code. ## Benchmark Benchmark results are in, looks like there's no measurable speed difference between 22.12 pre-boundary exclusion and our current implementation: ``` (rapids) rapids@compose:~/cuspatial/python/cuspatial/benchmarks$ pytest api/bench_api.py::bench_point_in_polygon ================================================== test session starts =================================================== platform linux -- Python 3.8.15, pytest-7.2.0, pluggy-1.0.0 benchmark: 4.0.0 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000) rootdir: /home/tcomer/mnt/NVIDIA/rapids-docker/cuspatial/python/cuspatial/benchmarks, configfile: pytest.ini plugins: cov-4.0.0, benchmark-4.0.0, cases-3.6.13, xdist-3.0.2, anyio-3.6.2, hypothesis-6.58.1 collected 1 item api/bench_api.py . [100%] ---------------------------------------------- benchmark: 1 tests --------------------------------------------- Name (time in s) Min Max Mean StdDev Median IQR Outliers OPS Rounds Iterations --------------------------------------------------------------------------------------------------------------- bench_point_in_polygon 1.9636 1.9749 1.9678 0.0043 1.9660 0.0045 1;0 0.5082 5 1 --------------------------------------------------------------------------------------------------------------- Legend: Outliers: 1 Standard Deviation from Mean; 1.5 IQR (InterQuartile Range) from 1st Quartile and 3rd Quartile. OPS: Operations Per Second, computed as 1 / Mean =================================================== 1 passed in 16.28s =================================================== (rapids) rapids@compose:~/cuspatial/python/cuspatial/benchmarks$ git status On branch feature/GeoSeries.contains ``` vs `branch-22.12` ``` (rapids) rapids@compose:~/cuspatial/python/cuspatial/benchmarks$ pytest api/bench_api.py::bench_point_in_polygon ================================== test session starts =================================== platform linux -- Python 3.8.15, pytest-7.2.0, pluggy-1.0.0 benchmark: 4.0.0 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000) rootdir: /home/tcomer/mnt/NVIDIA/rapids-docker/cuspatial/python/cuspatial/benchmarks, configfile: pytest.ini plugins: cov-4.0.0, benchmark-4.0.0, cases-3.6.13, xdist-3.0.2, anyio-3.6.2, hypothesis-6.58.1 collected 1 item api/bench_api.py . [100%] ---------------------------------------------- benchmark: 1 tests --------------------------------------------- Name (time in s) Min Max Mean StdDev Median IQR Outliers OPS Rounds Iterations --------------------------------------------------------------------------------------------------------------- bench_point_in_polygon 1.9516 1.9843 1.9730 0.0126 1.9760 0.0127 1;0 0.5068 5 1 --------------------------------------------------------------------------------------------------------------- Legend: Outliers: 1 Standard Deviation from Mean; 1.5 IQR (InterQuartile Range) from 1st Quartile and 3rd Quartile. OPS: Operations Per Second, computed as 1 / Mean =================================== 1 passed in 16.61s =================================== (rapids) rapids@compose:~/cuspatial/python/cuspatial/benchmarks$ git status On branch benchmark/branch-22.12 ``` ## Still adding: - [x] Detailed description of xfail result. - [x] Self-review existing `.contains` implementation in python. - [x] Update `.contains` docs when necessary. - [x] Benchmark again and document here. - [x] Move binops_with_quadtree.py to next branch. - [x] `.contains` Examples Authors: - H. Thomson Comer (https://github.com/thomcom) Approvers: - Michael Wang (https://github.com/isVoid) - Mark Harris (https://github.com/harrism) URL: #749

Create new branch with pairwise_point_in_polygon cpp changes.

121e146

thomcom requested review from a team as code owners October 24, 2022 17:39

thomcom requested review from robertmaynard, harrism and zhangjianting October 24, 2022 17:39

github-actions bot added cmake Related to CMake code or build configuration libcuspatial Relates to the cuSpatial C++ library Python Related to Python code labels Oct 24, 2022

thomcom added 2 commits October 24, 2022 12:43

Clean up docs on cursory review.

756282d

Use T as TypeParam

f5f7777

thomcom self-assigned this Oct 24, 2022

thomcom added 4 - Needs Reviewer Waiting for reviewer to review or respond feature request New feature or request non-breaking Non-breaking change labels Oct 24, 2022

thomcom added this to the DE-9IM milestone Oct 24, 2022

This was linked to issues Oct 24, 2022

[FEA] Determine if point-in-polygon is exterior inclusive or not. #738

Closed

Benchmark boundary exclusion #742

Closed

isVoid requested changes Oct 25, 2022

View reviewed changes

harrism requested changes Oct 25, 2022

View reviewed changes

cpp/include/cuspatial/experimental/detail/is_point_in_polygon_kernel.cuh Outdated Show resolved Hide resolved

thomcom added 4 commits October 25, 2022 08:37

Add epsilon to colinearity test.

a6f8f69

Add EPSILON and refactor some tests.

3a34016

Remove stale comment.

e8af434

Correct comments

70437da

harrism reviewed Oct 27, 2022

View reviewed changes

isVoid reviewed Oct 27, 2022

View reviewed changes

thomcom added 3 commits November 1, 2022 17:00

Rename is_point_in_polygon.cuh

c55ac72

Keep only colinearity epsilon.

5624068

Change to grid/stride loop.

7550399

harrism requested changes Nov 7, 2022

View reviewed changes

thomcom and others added 6 commits November 28, 2022 11:52

Merge branch 'branch-22.12' into feature/pairwise_point_in_polygon

8786614

Fix bug with grid-stride loop and switch to for comparison.

be5a5c0

Update cpp/include/cuspatial/pairwise_point_in_polygon.hpp

f7bbf10

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

Update cpp/include/cuspatial/experimental/pairwise_point_in_polygon.cuh

b6d55b8

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

Update cpp/include/cuspatial/experimental/pairwise_point_in_polygon.cuh

d3d7533

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

run-clang-format.py

0edd2df

This was referenced Nov 28, 2022

Wrap point_in_polygon iterators with permutation iterators #821

Open

Use a device functor with linear indexing with pairwise_point_in_polygon. #822

Open

Avoid vector<bool>, use thrust::host_vector<bool> instead. #823

Closed

harrism reviewed Nov 28, 2022

View reviewed changes

cpp/include/cuspatial/experimental/detail/pairwise_point_in_polygon.cuh Outdated Show resolved Hide resolved

harrism approved these changes Nov 28, 2022

View reviewed changes

isVoid reviewed Nov 28, 2022

View reviewed changes

isVoid mentioned this pull request Nov 28, 2022

Replace pairwise_point_in_polygon vector factories with make_device_vector. #825

Closed

isVoid approved these changes Nov 28, 2022

View reviewed changes

thomcom and others added 3 commits November 29, 2022 08:24

Improve grid stride loop computations.

1c02f3c

Update cpp/include/cuspatial/experimental/detail/is_point_in_polygon.cuh

89169ee

Co-authored-by: Michael Wang <isVoid@users.noreply.github.com>

Merge branch 'feature/pairwise_point_in_polygon' of github.com:thomco…

e5e937f

…m/cuspatial into feature/pairwise_point_in_polygon

thomcom mentioned this pull request Nov 29, 2022

Create GeoSeries.contains_properly method using point_in_polygon. #749

Merged

9 tasks

rapids-bot bot merged commit 924b570 into rapidsai:branch-22.12 Nov 29, 2022

thomcom mentioned this pull request Nov 30, 2022

[BUG] Point-in-polygon Primitive Should be Consistent Over Boundary Condition #744

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create `pairwise_point_in_polygon` to be used by pairwise `GeoSeries` #750

Create `pairwise_point_in_polygon` to be used by pairwise `GeoSeries` #750

thomcom commented Oct 24, 2022 •

edited

Loading

isVoid left a comment

harrism left a comment

harrism left a comment

isVoid left a comment

harrism commented Nov 7, 2022

isVoid left a comment •

edited

Loading

harrism commented Nov 29, 2022

Create pairwise_point_in_polygon to be used by pairwise GeoSeries #750

Create pairwise_point_in_polygon to be used by pairwise GeoSeries #750

Conversation

thomcom commented Oct 24, 2022 • edited Loading

Description

Checklist

isVoid left a comment

Choose a reason for hiding this comment

harrism left a comment

Choose a reason for hiding this comment

harrism left a comment

Choose a reason for hiding this comment

isVoid left a comment

Choose a reason for hiding this comment

harrism commented Nov 7, 2022

isVoid left a comment • edited Loading

Choose a reason for hiding this comment

harrism commented Nov 29, 2022

Create `pairwise_point_in_polygon` to be used by pairwise `GeoSeries` #750

Create `pairwise_point_in_polygon` to be used by pairwise `GeoSeries` #750

thomcom commented Oct 24, 2022 •

edited

Loading

isVoid left a comment •

edited

Loading