Feature/python benchmarking #11125
Feature/python benchmarking #11125
Conversation
vyasr
added
feature request
vyasr
requested review from
shwina and
isVoid
and removed request for
rgsl888prabhu
|
This PR has a large changeset by necessity. In order to make this review process a bit smoother, I have explicitly requested reviews from people who have contributed benchmarks before and/or are familiar with my design for the benchmarks (@isVoid, @shwina, and @galipremsagar). |
|
rerun tests |
Codecov Report
@@ Coverage Diff @@
## branch-22.08 #11125 +/- ##
===============================================
Coverage ? 86.33%
===============================================
Files ? 144
Lines ? 22751
Branches ? 0
===============================================
Hits ? 19641
Misses ? 3110
Partials ? 0 Continue to review full report at Codecov.
|
Done |
There was a problem hiding this comment.
Looks pretty good! This is a good base of benchmarks that can be expanded on in the future.
Not sure if this was discussed, but how did you imagine these benchmarks to be used? For example in pandas, we have used ASV to catch performance regressions across verions, but we're inconsistent about this as it takes a while to run the whole suite.
|
rerun tests |
We have indeed discussed this before. ASV is something that we've definitely considered, not for running benchmarks but rather using the dashboards with data from pytest-benchmark via some plugins written by other RAPIDS team members. For the moment, the process of tracking historical benchmark results is pretty manual. We've discussed automating benchmark runs more going forward, but we haven't come to any specific solution yet. I think that to some extent this is waiting on various changes from our ops team. |
|
rerun tests |
…tive imports." This reverts commit 5db4589.
|
@bdice I had to roll back the changes making the benchmarks a Python package because it appears to change pytest's package discovery logic in a way that leads to it always finding the local copy of the package rather than the installed one, which is problematic when the package is not built in place (the case in our CI). I'm happy to revisit this further in the future, but in the interest of moving us forward I would like to merge as is and try to improve the organization and address the CI issues in future PRs. |
|
@gpucibot merge |
The version of `bench_isin` merged in #11125 used key and column names of the format `f"key{i}"` rather than the format `f"{string.ascii_lowercase[i]}"` as is used in the dataframe generator. As a result the `isin` benchmark using a dictionary argument short-circuits with no matching keys, and the `isin` benchmark using a dataframe argument finds no matches. This PR also adjusts the `isin` arguments from `range(1000)` to `range(50)` to better match the input dataframe cardinality of 100. With `range(1000)`, every element matches but with `range(50)` only 50% of the elements match. Authors: - Gregory Kimball (https://github.com/GregoryKimball) Approvers: - Bradley Dice (https://github.com/bdice) - GALI PREM SAGAR (https://github.com/galipremsagar) URL: #11549
This PR ports the benchmarks in https://github.com/vyasr/cudf_benchmarks, adding official benchmarks to the repository. The new benchmarks are designed from the ground up to make the best use of pytest, pytest-benchmark, and pytest-cases to simplify writing and maintaining benchmarks. Extended discussions of various previous design questions may be found on the original repo. Reviewers may also benefit from reviewing the companion PR creating documentation for how to write benchmarks, #11122.
Tests will not pass here until rapidsai/integration#492 is merged.