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

Remove GDAL dependency in quadtree spatial join tests. #974

Merged
merged 4 commits into from
Mar 13, 2023

Conversation

harrism
Copy link
Member

@harrism harrism commented Mar 7, 2023

Description

Closes #958. Replaces usage of GDAL contains in the quadtree point-in-polygon tests with a call to a different cuSpatial point-in-polygon function. This allows us to remove all current dependencies on GDAL in cuSpatial.

Depends on #973. This PR currently updates some expectations to make tests pass that are fixed in that PR.

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 cmake Related to CMake code or build configuration conda Related to conda and conda configuration libcuspatial Relates to the cuSpatial C++ library labels Mar 7, 2023
@harrism harrism changed the title Fea-remove-gdal-from-test Remove GDAL dependency in quadtree spatial join tests. Mar 7, 2023
@harrism harrism self-assigned this Mar 7, 2023
@harrism harrism added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Mar 7, 2023
@harrism harrism marked this pull request as ready for review March 7, 2023 06:03
@harrism harrism requested review from a team as code owners March 7, 2023 06:03
Copy link
Contributor

@isVoid isVoid left a comment

Choose a reason for hiding this comment

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

No more gdal dependency!

std::uint32_t bits = hits_host[point_index];
while (bits != 0) {
std::uint32_t t = bits & -bits; // get only LSB
std::uint32_t poly_index = __builtin_ctzl(bits); // get index of LSB
Copy link
Contributor

Choose a reason for hiding this comment

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

Since it's int type here, shouldn't the most compatible function is __builtin_ctz instead of __builtin_ctzl?

@harrism harrism added the 5 - Merge After Dependencies Depends on another PR: do not merge out of order label Mar 8, 2023
Copy link
Contributor

@trxcllnt trxcllnt left a comment

Choose a reason for hiding this comment

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

LGTM, just left a small comment for discussion.

@harrism
Copy link
Member Author

harrism commented Mar 9, 2023

/merge

@rapids-bot rapids-bot bot merged commit 66327f5 into rapidsai:branch-23.04 Mar 13, 2023
trxcllnt pushed a commit to trxcllnt/cuspatial that referenced this pull request Mar 15, 2023
Closes rapidsai#958.  Replaces usage of GDAL `contains` in the quadtree point-in-polygon tests with a call to a different cuSpatial point-in-polygon function. This allows us to remove all current dependencies on GDAL in cuSpatial.

Depends on rapidsai#973. This PR currently updates some expectations to make tests pass that are fixed in that PR.

Authors:
  - Mark Harris (https://github.com/harrism)

Approvers:
  - Michael Wang (https://github.com/isVoid)
  - H. Thomson Comer (https://github.com/thomcom)
  - Paul Taylor (https://github.com/trxcllnt)
  - Jordan Jacobelli (https://github.com/jjacobelli)

URL: rapidsai#974
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Merge After Dependencies Depends on another PR: do not merge out of order cmake Related to CMake code or build configuration conda Related to conda and conda configuration improvement Improvement / enhancement to an existing function 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.

[FEA]: Remove dependency on GDAL in libcuspatial point-in-polygon tests
5 participants