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

Pairwise Point to Point Distance; Rename Folder distances to distance #558

Merged
merged 50 commits into from
Jul 29, 2022

Conversation

isVoid
Copy link
Contributor

@isVoid isVoid commented Jun 8, 2022

Implements 2D point-point L2 distance. Also renamed folder distances into distance

Contributes to #231

@isVoid isVoid requested a review from a team as a code owner June 8, 2022 22:40
@isVoid isVoid changed the title Pairwise point to point distance Pairwise point to point distance; rename folder distances to distance Jul 28, 2022
@isVoid isVoid changed the title Pairwise point to point distance; rename folder distances to distance Pairwise Point to Point Distance; Rename Folder distances to distance Jul 28, 2022
Copy link
Contributor

@robertmaynard robertmaynard left a comment

Choose a reason for hiding this comment

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

Approving CMake 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.

Looks good. A couple of missing tests and other nits.

cpp/benchmarks/hausdorff_benchmark.cpp Show resolved Hide resolved
cpp/include/cuspatial/distance/point_distance.hpp Outdated Show resolved Hide resolved
Comment on lines +39 to +46
static_assert(
detail::is_floating_point<T,
typename std::iterator_traits<Cart2dItB>::value_type::value_type,
typename std::iterator_traits<OutputIt>::value_type>(),
"Inputs and output must be floating point types.");

static_assert(detail::is_same<T, typename std::iterator_traits<OutputIt>::value_type>(),
"Inputs and output must be the same types.");
Copy link
Member

Choose a reason for hiding this comment

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

If we merge #579 you can replace this with is_same_floating_point.

cpp/tests/experimental/spatial/point_distance_test.cu Outdated Show resolved Hide resolved
cpp/tests/spatial/point_distance_test.cpp Show resolved Hide resolved
@harrism harrism added breaking Breaking change and removed non-breaking Non-breaking change labels Jul 28, 2022
@harrism
Copy link
Member

harrism commented Jul 28, 2022

Also need to update paths in Cython files to fix the build.

@isVoid isVoid requested a review from a team as a code owner July 29, 2022 01:12
@github-actions github-actions bot added the Python Related to Python code label Jul 29, 2022
@isVoid
Copy link
Contributor Author

isVoid commented Jul 29, 2022

Our cython file org fell out of sync with cpp file org. So in this PR I also moved them into spatial module.

@isVoid isVoid requested a review from harrism July 29, 2022 01:13
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 @isVoid !

@isVoid
Copy link
Contributor Author

isVoid commented Jul 29, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 487dd88 into rapidsai:branch-22.08 Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change cmake Related to CMake code or build configuration feature request New feature or request libcuspatial Relates to the cuSpatial C++ library Python Related to Python code
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants