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.

Sign up for GitHub

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

Jump to bottom

Python Bindings for Pairwise Linestring Distance #521

Merged
merged 101 commits into from
Jun 1, 2022
Merged

Python Bindings for Pairwise Linestring Distance #521

merged 101 commits into from
Jun 1, 2022

Conversation

isVoid
Copy link
Contributor

@isVoid isVoid commented May 6, 2022

This PR introduces python bindings for pairwise linestring distance. Depends on #510 and has lots of commits from that PR. Should reduce once #510 merges.

isVoid and others added 30 commits March 31, 2022 15:16
@isVoid
Initial pass of linestring distance and test
0f2c62c
@isVoid
Add more one pair linestring tests
29f1d62
@isVoid
More single pair testing
954bbfc
@isVoid
update docstring
03748ea
@isVoid
Add medium test
3b52203
@isVoid
Wrapping debug prints
6deb261
@isVoid
Initial
61716c6
@isVoid
Merge branch 'fea/enable_nvbench' into feature/linestring_distance
95a803f
@isVoid
Merge branch 'branch-22.06' of github.com:rapidsai/cuspatial into fea…
3375b1b 
…ture/linestring_distance_benchmark
@isVoid
initial
3cab72d
@isVoid
Merge branch 'fix/update_cuda_try' into feature/linestring_distance_b…
1c6322e 
…enchmark
@isVoid
Update synchonization.cpp
f6b571a
@isVoid
Merge branch 'fix/update_cuda_try' into fea/enable_nvbench
2c3a038
@isVoid
Replace one more usage of CUDF_CUDA_TRY
90de8ce
@isVoid
Merge branch 'fix/update_cuda_try' into feature/linestring_distance_b…
3efb6b0 
…enchmark
@isVoid
Merge branch 'fea/enable_nvbench' into feature/linestring_distance_be…
a637664 
…nchmark
@isVoid
Add nvbench cpm
d82f887
@isVoid
Merge branch 'fea/enable_nvbench' into feature/linestring_distance_be…
80e2d27 
…nchmark
@isVoid
Add runnable nvbench
813178c
@isVoid
Add data gen synchronizer
aa65c75
@isVoid
Rewrites kernel to schedule on num points
511dd5d
@isVoid
Change problem size in benchmarks
cee2c1f
@isVoid
In the middle of getting the compiler to include the file.
3bf3d44
@isVoid
passes compilation
2006ac1
@isVoid
Revert "passes compilation"
f463bd8 
This reverts commit 2006ac1.
@isVoid
Revert "In the middle of getting the compiler to include the file."
08ef028 
This reverts commit 3bf3d44.
@isVoid
Code cleanups and completes docstrings
53e822a
@isVoid
Revert "Change problem size in benchmarks"
5dd6a4c 
This reverts commit cee2c1f.
@isVoid
Revert "Add data gen synchronizer"
2b1da4a 
This reverts commit aa65c75.
@isVoid
Revert "Add runnable nvbench"
d8e6902 
This reverts commit 813178c.
@github-actions github-actions bot removed the libcuspatial Relates to the cuSpatial C++ library label May 12, 2022
@isVoid isVoid added feature request New feature or request non-breaking Non-breaking change labels May 12, 2022
@isVoid
Copy link
Contributor Author

isVoid commented May 12, 2022

Note - the doc entry of this method will be added after #523 is merged.

harrism
harrism requested changes May 26, 2022
View reviewed changes
Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

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

Just some suggestions on expanding the documentation and making it match the C++ API, since the behavior should be the same.

python/cuspatial/cuspatial/_lib/linestring_distance.pyx Show resolved Hide resolved
python/cuspatial/cuspatial/core/gis.py Outdated Show resolved Hide resolved
python/cuspatial/cuspatial/core/gis.py Outdated Show resolved Hide resolved
python/cuspatial/cuspatial/core/gis.py Show resolved Hide resolved
@harrism harrism requested review from thomcom and removed request for zhangjianting May 26, 2022 04:22
@github-actions github-actions bot added the libcuspatial Relates to the cuSpatial C++ library label May 26, 2022
@isVoid isVoid requested a review from harrism May 27, 2022 00:58
thomcom
thomcom approved these changes May 27, 2022
View reviewed changes
Copy link
Contributor

@thomcom thomcom left a comment

Choose a reason for hiding this comment

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

lg2m

@vyasr
Copy link
Contributor

vyasr commented May 28, 2022

@isVoid I'm pretty sure the test failures here came from CI merging in my scikit-build changes from branch-22.06 down into your branch and then attempting to compile the resulting files. Since the new builds require adding the Python files to the CMakeLists.txt, your new module wasn't getting compiled. I merged in branch-22.06 and added the new file, which hopefully should fix the issue.

@harrism
Copy link
Member

harrism commented May 30, 2022

RuntimeError: cuSpatial failure at: /workspace/.conda-bld/work/cpp/src/spatial/linestring_distance.cu:292: Mismatch number of linestrings in the linestring pair array.

@harrism
Copy link
Member

harrism commented Jun 1, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit da1cb97 into rapidsai:branch-22.06 Jun 1, 2022
@isVoid isVoid mentioned this pull request Jun 22, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harrism harrism harrism approved these changes

@thomcom thomcom thomcom approved these changes

Assignees

@isVoid isVoid

Labels
cmake Related to CMake code or build configuration feature request New feature or request libcuspatial Relates to the cuSpatial C++ library non-breaking Non-breaking change Python Related to Python code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@isVoid @vyasr @harrism @thomcom