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

Improve Hausdorff Many Column Performance #916

Merged
merged 12 commits into from
Apr 11, 2023
Merged

Improve Hausdorff Many Column Performance #916

merged 12 commits into from
Apr 11, 2023

Conversation

isVoid
Copy link
Contributor

@isVoid isVoid commented Feb 10, 2023 •
edited
Loading

Description

This PR changes the C++ hausdorff distance interface to return the owning item that contains the interface and a table view into the result. This avoids a numba reshape that slows performance when there are many columns.

Python API benchmark (10K input spaces)
branch-23.06:

--------------------------------------------------------- benchmark: 1 tests ---------------------------------------------------------
Name (time in s)                                     Min     Max    Mean  StdDev  Median     IQR  Outliers     OPS  Rounds  Iterations
--------------------------------------------------------------------------------------------------------------------------------------
bench_directed_hausdorff_distance_many_spaces     1.4473  1.4938  1.4688  0.0168  1.4655  0.0175       2;0  0.6808       5           1
--------------------------------------------------------------------------------------------------------------------------------------

this pr:

-------------------------------------------------------------- benchmark: 1 tests --------------------------------------------------------------
Name (time in ms)                                      Min       Max      Mean   StdDev    Median      IQR  Outliers     OPS  Rounds  Iterations
------------------------------------------------------------------------------------------------------------------------------------------------
bench_directed_hausdorff_distance_many_spaces     221.5014  288.3597  237.1724  28.7577  223.4605  21.0320       1;1  4.2163       5           1
------------------------------------------------------------------------------------------------------------------------------------------------

6.5x speedup.

contributes to #1013

The CPP benchmarks shows no significant performance regression:

Comparing branch2304.json to after.json
Benchmark                                                           Time             CPU      Time Old      Time New       CPU Old       CPU New
------------------------------------------------------------------------------------------------------------------------------------------------
HausdorffBenchmark/hausdorff/32/4/manual_time                    -0.0369         -0.0519             0             0             1             0
HausdorffBenchmark/hausdorff/64/4/manual_time                    -0.0245         -0.0480             0             0             1             1
HausdorffBenchmark/hausdorff/512/4/manual_time                   -0.0173         -0.0125             1             1             1             1
HausdorffBenchmark/hausdorff/4096/4/manual_time                  +0.0603         +0.1068             8             8             8             9
HausdorffBenchmark/hausdorff/8192/4/manual_time                  -0.0256         -0.0240            18            18            18            18
HausdorffBenchmark/hausdorff/32/8/manual_time                    -0.0068         +0.0780             0             0             1             1
HausdorffBenchmark/hausdorff/64/8/manual_time                    -0.0168         -0.0213             0             0             1             1
HausdorffBenchmark/hausdorff/512/8/manual_time                   -0.0216         -0.0242             1             1             2             2
HausdorffBenchmark/hausdorff/4096/8/manual_time                  -0.0296         -0.0092            12            12            13            13
HausdorffBenchmark/hausdorff/8192/8/manual_time                  +0.0314         +0.0328            28            29            28            29
HausdorffBenchmark/hausdorff/32/64/manual_time                   -0.0139         -0.0879             0             0             1             1
HausdorffBenchmark/hausdorff/64/64/manual_time                   -0.0001         +0.0343             0             0             1             1
HausdorffBenchmark/hausdorff/512/64/manual_time                  -0.0935         -0.0816             5             5             6             6
HausdorffBenchmark/hausdorff/4096/64/manual_time                 -0.0299         -0.0295           186           180           187           181
HausdorffBenchmark/hausdorff/8192/64/manual_time                 -0.1097         -0.1094           685           610           685           610
HausdorffBenchmark/hausdorff/32/128/manual_time                  +0.0065         +0.1026             0             0             1             1
HausdorffBenchmark/hausdorff/64/128/manual_time                  +0.0035         -0.0169             1             1             1             1
HausdorffBenchmark/hausdorff/512/128/manual_time                 -0.0312         -0.0299            10            10            11            10
HausdorffBenchmark/hausdorff/4096/128/manual_time                -0.0388         -0.0388           613           589           613           589
HausdorffBenchmark/hausdorff/8192/128/manual_time                -0.0080         -0.0080          2353          2335          2354          2335

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 February 10, 2023 05:19
@github-actions github-actions bot added libcuspatial Relates to the cuSpatial C++ library Python Related to Python code labels Feb 10, 2023
@isVoid isVoid added breaking Breaking change improvement Improvement / enhancement to an existing function labels Feb 10, 2023
@isVoid isVoid marked this pull request as draft February 10, 2023 05:40
@isVoid
Copy link
Contributor Author

isVoid commented Feb 10, 2023

Planning to address #885 in this PR too. Switching to draft.

@isVoid isVoid requested review from thomcom and harrism and removed request for zhangjianting February 13, 2023 21:20
@isVoid isVoid marked this pull request as ready for review February 13, 2023 21:21
@isVoid isVoid added the 3 - Ready for Review Ready for review by team label Feb 13, 2023
@isVoid
Copy link
Contributor Author

isVoid commented Feb 13, 2023

This is now ready for review.

harrism
harrism requested changes Feb 14, 2023
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.

Need more explanation of this in the documentation.

cpp/src/spatial/hausdorff.cu Outdated Show resolved Hide resolved
cpp/src/spatial/hausdorff.cu Outdated Show resolved Hide resolved
cpp/include/cuspatial/distance/hausdorff.hpp Show resolved Hide resolved
@isVoid isVoid requested review from harrism and thomcom February 24, 2023 22:51
@isVoid isVoid changed the base branch from branch-23.04 to branch-23.06 April 11, 2023 00:38
@isVoid
Copy link
Contributor Author

isVoid commented Apr 11, 2023

/merge

@rapids-bot rapids-bot bot merged commit 062f0e3 into rapidsai:branch-23.06 Apr 11, 2023
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

@thomcom thomcom thomcom approved these changes

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

Assignees
No one assigned
Labels
3 - Ready for Review Ready for review by team breaking Breaking change improvement Improvement / enhancement to an existing function libcuspatial Relates to the cuSpatial C++ library Python Related to Python code
Projects
cuSpatial
Status: Todo
Milestone
No milestone
Development

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