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.

Sign up for GitHub

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

Jump to bottom

[FEA] Fuse transform and copy_if operations in quadtree_point_in_polygon #559

Merged
merged 4 commits into from
Jun 22, 2022
Merged

[FEA] Fuse transform and copy_if operations in quadtree_point_in_polygon #559

merged 4 commits into from
Jun 22, 2022

Conversation

trxcllnt
Copy link
Contributor

Fuse transform and copy_if operations in quadtree_point_in_polygon for a modest performance improvement.

branch-22.08:

points in polygons (168,898,952 points, 788 polygons): 1.669s

(168898952 * 788 / 1669.1560840010643 * 1000) =
  79,736,326,309.862 (point-polygon pairs / second)

This PR:

points in polygons (168,898,952 points, 788 polygons): 1.569s

(168898952 * 788 / 1569.104240000248 * 1000) =
  84,820,607,059.209 (point-polygon pairs / second)

@trxcllnt trxcllnt requested a review from a team as a code owner June 14, 2022 14:48
@github-actions github-actions bot added the libcuspatial Relates to the cuSpatial C++ library label Jun 14, 2022
@trxcllnt trxcllnt added 3 - Ready for Review Ready for review by team improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jun 14, 2022
isVoid
isVoid approved these changes Jun 14, 2022
View reviewed changes
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.

I don't know the whole process of the quadtree PIP algorithm yet but I can see the improvement here combines the counting iterator and the transform call into a counting transform iterator and reduces a kernel call.

cpp/src/join/quadtree_point_in_polygon.cu Outdated Show resolved Hide resolved
@harrism
Copy link
Member

harrism commented Jun 15, 2022

What GPU are the perf numbers from?

harrism
harrism approved these changes Jun 15, 2022
View reviewed 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.

Fusion!

cpp/src/join/quadtree_point_in_polygon.cu Show resolved Hide resolved
@trxcllnt
Copy link
Contributor Author

rerun tests

@trxcllnt
Copy link
Contributor Author

@harrism Those numbers are from running the NYC taxi dataset benchmark on my local Quadro 8k.

@isVoid
Copy link
Contributor

isVoid commented Jun 15, 2022

rerun tests

@trxcllnt trxcllnt requested a review from harrism June 22, 2022 01:54
harrism
harrism approved these changes Jun 22, 2022
View reviewed 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.

I still approve. :)

Once you have A100 benchmark comparison, please add here.

@harrism
Copy link
Member

harrism commented Jun 22, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 5e34527 into rapidsai:branch-22.08 Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harrism harrism harrism approved these changes

@isVoid isVoid isVoid approved these changes

@zhangjianting zhangjianting Awaiting requested review from zhangjianting zhangjianting is a code owner automatically assigned from rapidsai/cuspatial-cpp-codeowners

Assignees
No one assigned
Labels
3 - Ready for Review Ready for review by team improvement Improvement / enhancement to an existing function libcuspatial Relates to the cuSpatial C++ library non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@trxcllnt @harrism @isVoid