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 Header Only API pairwise_linestring_intersection #852

Merged

Conversation

isVoid
Copy link
Contributor

@isVoid isVoid commented Dec 10, 2022

Description

This PR adds header only API pairwise_linestring_intersection. Closes #765 .

Depends on #813 #818 #819 #851

Checklist

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

…into feature/intersection_with_duplicates_pr
…into feature/intersection_with_duplicates_pr
@isVoid
Copy link
Contributor Author

isVoid commented Dec 14, 2022

Do we have a concern about dropping duplicates and making it harder for the user to identify which points intersect or their count?

Unfortunately, the original requirement for the output of intersection is unclear about what the user really expects here. I went afar to return the most cleaned-up output that the algorithm can return. (So does geopandas) It could be an early-optimization. It could be likely that it's not what the user wants. If you think the design is not meeting the need, feel free to file an issue to track.

My question is, does this intersection function help your binary predicate use case?

isVoid and others added 4 commits December 14, 2022 11:13
…diates_remove_if_test.cu

Co-authored-by: H. Thomson Comer <thomcom@gmail.com>
…id/cuspatial into feature/header_only_intersection/pr
…into feature/header_only_intersection/pr
…into feature/header_only_intersection/pr
@thomcom
Copy link
Contributor

thomcom commented Dec 15, 2022

I'm sure this helps with implementing a great deal of binary predicates! I'm not exactly sure how, yet. I'm a little worried about a case of $\forall p\ \in \lambda_1 \cap \lambda_2$ if it is required. This definitely covers None and any cases, which, without looking closely at dozens of intersection predicate calls yet, probably covers most of them.

@thomcom
Copy link
Contributor

thomcom commented Dec 15, 2022

My questions are mostly general and for the purpose of understanding what's coming my way soon. :) I see @harrism already approved, I will approve if you want to get this merged before the week ends.

@isVoid
Copy link
Contributor Author

isVoid commented Dec 15, 2022

@gpucibot merge

@isVoid isVoid added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 2 - In Progress Currenty a work in progress labels Dec 15, 2022
@rapids-bot rapids-bot bot merged commit f945e67 into rapidsai:branch-23.02 Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge 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
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

header-only implementation of intersection
3 participants