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 find_and_combine_segment internal API #818

Merged
merged 11 commits into from
Dec 13, 2022

Conversation

isVoid
Copy link
Contributor

@isVoid isVoid commented Nov 23, 2022

Description

This PR adds find_and_combine_segement API. For each set of segments, this API writes the first mergeable segment with the merged result and set the flags of the rest merged segments to 1.

Closes #816

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 isVoid requested review from a team as code owners November 23, 2022 01:23
@isVoid isVoid requested review from zhangjianting, trxcllnt and harrism and removed request for trxcllnt and zhangjianting November 23, 2022 01:23
@github-actions github-actions bot added cmake Related to CMake code or build configuration libcuspatial Relates to the cuSpatial C++ library labels Nov 23, 2022
@isVoid isVoid requested a review from thomcom November 23, 2022 01:23
@isVoid isVoid self-assigned this Nov 23, 2022
@isVoid isVoid added 3 - Ready for Review Ready for review by team feature request New feature or request non-breaking Non-breaking change labels Nov 23, 2022
@isVoid isVoid changed the base branch from branch-22.12 to branch-23.02 November 28, 2022 17:47
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.

Looking good.

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.

Also LGTM.

@isVoid isVoid requested a review from harrism December 8, 2022 21:57
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.

Nice work!

}

/// Return the geometric center of segment.
vec_2d<T> CUSPATIAL_HOST_DEVICE center() const { return midpoint(first, second); }
Copy link
Member

Choose a reason for hiding this comment

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

Would midpoint() be a better name?

Copy link
Contributor Author

@isVoid isVoid Dec 13, 2022

Choose a reason for hiding this comment

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

Aha! I thought about this too, and I think center is a more generic name for the geometric-center of the geometry. Think about deconditioning the floating point coordinates as inversely translating the object with the -center vector, do op and translate back. You can apply this sequence regardless of type of geometry you are dealing with. A box/circle may also have a geometry center and adopt the same sequence, albeit the implementation of .center is different. I imagine .center to exist in part of some super class Geometry2D that will cover everything from point, segment, linestring_ref, polygon_ref. .center is virtual function that can be implemented by all inherited classes.

@isVoid
Copy link
Contributor Author

isVoid commented Dec 13, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 222739f into rapidsai:branch-23.02 Dec 13, 2022
rapids-bot bot pushed a commit that referenced this pull request Dec 16, 2022
This PR adds header only API `pairwise_linestring_intersection`. Closes #765 .

Depends on #813 #818 #819 #851

Authors:
  - Michael Wang (https://github.com/isVoid)

Approvers:
  - Mark Harris (https://github.com/harrism)
  - H. Thomson Comer (https://github.com/thomcom)

URL: #852
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team 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.

Create Internal API to Merge Segments for All Segments in the Same Space
3 participants