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

[BUG] Removed unnecessary device-to-host copy which caused a performance regression #1101

Merged

Conversation

rlratzel
Copy link
Contributor

@rlratzel rlratzel commented Aug 19, 2020

  • Removed unnecessary device-to-host copy which caused a performance regression
  • Fixed some search-and-replace errors in comments
  • Added more detail about k to the BC docstring

Verified performance regression was gone using the benchmarks:

> pytest -v bench_algos.py::bench_betweenness_centrality[ds=../datasets/csv/directed/cit-Patents.csv,mm=True,pa=True] --benchmark-autosave --benchmark-compare=0001

-------------------------------------------------------------------------------------------------------------------- benchmark: 2 tests ---------------------------------------------------------
Name (time in ms)                                                                                                Min                Max               Mean            StdDev            Outliers
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
bench_betweenness_centrality[ds=../datasets/csv/directed/cit-Patents.csv,mm=True,pa=True] (NOW)              30.3700 (1.0)      30.6144 (1.0)      30.4473 (1.0)      0.0798 (1.0)           5;0
bench_betweenness_centrality[ds=../datasets/csv/directed/cit-Patents.csv,mm=True,pa=True] (0001_c90f5dc)     89.8043 (2.96)     90.5968 (2.96)     90.4228 (2.97)     0.2045 (2.56)          1;1
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

…gression. Fixed some search-and-replace errors in comments, added more detail about k to the BC docstring.
@rlratzel rlratzel requested review from a team as code owners August 19, 2020 22:15
@rlratzel rlratzel self-assigned this Aug 19, 2020
@GPUtester
Copy link
Contributor

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

@BradReesWork BradReesWork added this to the 0.15 milestone Aug 19, 2020
@codecov-commenter
Copy link

codecov-commenter commented Aug 19, 2020

Codecov Report

Merging #1101 into branch-0.15 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           branch-0.15    #1101   +/-   ##
============================================
  Coverage        72.12%   72.12%           
============================================
  Files               56       56           
  Lines             2102     2102           
============================================
  Hits              1516     1516           
  Misses             586      586           
Impacted Files Coverage Δ
...ython/cugraph/centrality/betweenness_centrality.py 98.03% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c90f5dc...f44a94e. Read the comment docs.

@BradReesWork BradReesWork merged commit d63bc9e into rapidsai:branch-0.15 Aug 20, 2020
@rlratzel rlratzel deleted the branch-0.15-bcregressionfix branch December 9, 2020 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants