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 multilinestring_segment_manager for segment related methods in multilinestring ranges #1134

Merged
merged 30 commits into from
Jun 1, 2023

Conversation

isVoid
Copy link
Contributor

@isVoid isVoid commented May 11, 2023

Description

closes #1055

This PR adds multilinestring_segment_manager owning class to track the lifetime of intermediate allocations that has to do with segment iterators. In addition multilinestring_segment_range is added as the non-owning class to the previous class to provide iterators to the segments. The syntax to use this object is like:

auto segment_manager = multilinestrings._segments(stream);
auto segment_range = segment_manager.range();

Then a user can easily access the segments in the multilinestrings with:

for (auto seg : segment_range) {
    auto length = sqrt(dot(seg.a, seg.b));
}

Secondly, this PR includes tests that has empty geometry collections in the input, linestring_polygon_distance now correctly computes nans for these input pairs.

In addition, CUSPATIAL_EXPECTS_VALID_MULTI*_SIZES is relaxed. Valid polygon and ring sizes are only implicitly required but not checked via the size of the arrays since they are insufficient checks. Accordingly, some tests are also relaxed.

Checklist

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

@github-actions github-actions bot added the libcuspatial Relates to the cuSpatial C++ library label May 11, 2023
@isVoid isVoid self-assigned this May 11, 2023
@harrism harrism added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 17, 2023
@isVoid isVoid changed the title Add segment_method (tentative name) for segment related methods in geometry ranges Add segment_method (tentative name) for segment related methods in geometry ranges. Refactors linestring_polygon_distance to partially support inputs that has empty linestring or empty multipolygons May 19, 2023
@isVoid isVoid changed the title Add segment_method (tentative name) for segment related methods in geometry ranges. Refactors linestring_polygon_distance to partially support inputs that has empty linestring or empty multipolygons Add multilinestring_segment for segment related methods in multilinestring ranges. Refactors linestring_polygon_distance to partially support inputs that has empty linestring or empty multipolygons May 19, 2023
@isVoid isVoid changed the title Add multilinestring_segment for segment related methods in multilinestring ranges. Refactors linestring_polygon_distance to partially support inputs that has empty linestring or empty multipolygons Add multilinestring_segment for segment related methods in multilinestring ranges May 19, 2023
@isVoid isVoid marked this pull request as ready for review May 19, 2023 17:43
@isVoid isVoid requested a review from a team as a code owner May 19, 2023 17:43
@isVoid isVoid requested review from trxcllnt and harrism May 19, 2023 17:43
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 algorithmic improvement. Some doc suggestions, some questions.

@isVoid isVoid changed the title Add multilinestring_segment for segment related methods in multilinestring ranges Add multilinestring_segment_manager for segment related methods in multilinestring ranges May 31, 2023
@isVoid isVoid requested a review from harrism May 31, 2023 17:45
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.

Thanks for the renamings. Looks good.

* segment iterators, call `segment_range()` function to create a non-owning object of this class.
*
* For detailed explanation on the implementation of the segment iterators, see documentation
* of `multilinestring_segment_range`.
Copy link
Member

Choose a reason for hiding this comment

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

Does quoting documented symbol names prevent Doxygen from making them into clickable links?

@isVoid
Copy link
Contributor Author

isVoid commented Jun 1, 2023

/merge

@rapids-bot rapids-bot bot merged commit 50e3b6a into rapidsai:branch-23.06 Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function libcuspatial Relates to the cuSpatial C++ library non-breaking Non-breaking change
Projects
Status: Review
Development

Successfully merging this pull request may close these issues.

[BUG]: Segment Iterator is flawed, may fail if input contains 0-length geometry
2 participants