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

[FEA] Use nvbench for new benchmarks and transition old benchmarks to use nvbench #7960

Closed
jrhemstad opened this issue Apr 14, 2021 · 9 comments
Labels
feature request New feature or request good first issue Good for newcomers improvement Improvement / enhancement to an existing function

Comments

@jrhemstad
Copy link
Contributor

Is your feature request related to a problem? Please describe.

nvbench is a new library for writing benchmarks of GPU accelerated applications. It's similar to Google Benchmark, but has several quality of life improvements when doing GPU benchmarking, e.g., it eliminates the need for us to do manual timing in GBench with the raii_event_timer object. It also can show things like % of peak BW achieved and other details about the GPU hardware.

Describe the solution you'd like

We have quite a few benchmarks right now, so it would take a while to transition everything to use nvbench. That said, any new benchmarks can and should be written with nvbench.

@jrhemstad jrhemstad added feature request New feature or request good first issue Good for newcomers tech debt improvement Improvement / enhancement to an existing function labels Apr 14, 2021
@harrism
Copy link
Member

harrism commented Apr 28, 2021

Oops, I've added several new benchmarks in the past two weeks before I saw this.

@harrism
Copy link
Member

harrism commented Apr 28, 2021

I think before we can switch nvbench would need a replacement for gbench's compare.py, which we rely on heavily.

@jrhemstad
Copy link
Contributor Author

I think before we can switch nvbench would need a replacement for gbench's compare.py, which we rely on heavily.

Yeah, @allisonvacanti has that on the P0 list already: NVIDIA/nvbench#9

@vyasr
Copy link
Contributor

vyasr commented Dec 7, 2021

I think before we can switch nvbench would need a replacement for gbench's compare.py, which we rely on heavily.

Yeah, @allisonvacanti has that on the P0 list already: NVIDIA/nvbench#9

Do we know what next steps are on that? @shwina and I worked on getting a version of compare.py ready and it was merged a while ago, but I don't recall how much more work is necessary to bring it to feature parity with gbench's compare.py so that we can close NVIDIA/nvbench#9. I can follow up if we don't.

@karthikeyann
Copy link
Contributor

I think before we can switch nvbench would need a replacement for gbench's compare.py, which we rely on heavily.

This is implemented in NVIDIA/nvbench#14

@karthikeyann
Copy link
Contributor

karthikeyann commented Dec 7, 2021

Basic version is fine. Documentation is required. CLI could be improved.
benchmarksfiltered feature might be needed.

@vyasr
Copy link
Contributor

vyasr commented Jan 5, 2022

@harrism @jrhemstad Now that we're trying to get nightly benchmarks going I think we'll need to prioritize this conversion so that we can standardize the data that will be available and start building more tooling for our dashboards. I'm going to follow up on the comparison script work now to see what next steps are there.
CC @GregoryKimball @randerzander @robertmaynard

@bdice
Copy link
Contributor

bdice commented Jan 20, 2022

I updated our Unit Benchmarking in libcudf Guide to reflect the policy that new benchmarks should use NVBench in PR #10093. I think most of the information there still holds.

rapids-bot bot pushed a commit that referenced this issue Jan 24, 2022
This PR relates to #7960 and indicates that new benchmarks should be written using [NVBench](https://github.com/NVIDIA/nvbench). (This PR does not resolve that issue because it does not transition any existing benchmarks to nvbench, it is only an update in the developer guide.)

Authors:
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - Mark Harris (https://github.com/harrism)

URL: #10093
@vyasr
Copy link
Contributor

vyasr commented Jul 18, 2022

At this point PR reviewers are reasonably consistent at requiring that new benchmark code use NVbench or convert existing benchmarks. We should continue to encourage that so that we achieve a gradual transition. The infrastructure for NVbench is also all in place, and the benchmarking docs indicate that NVbench is our preferred tool. If we decide to make a concerted push to convert all remaining benchmarks to NVBench, we can reopen this issue or open a new one as a checklist of files to convert, but for now I'm going to close this as it's not particularly actionable.

@vyasr vyasr closed this as completed Jul 18, 2022
rapids-bot bot pushed a commit that referenced this issue Aug 23, 2022
Issue #10941

This PR rewrites the existing ORC reader benchmarks with nvbench w.r.t. #7960. It improves the `input` test case in which all data types were benchmarked with all compression and IO types. By splitting `input` into `decode` and `io_compression`, it reduces the number of test cases from 112 to 44.

The PR also removes the current `row_selection` test suite.

Authors:
  - Yunsong Wang (https://github.com/PointKernel)

Approvers:
  - Nghia Truong (https://github.com/ttnghia)
  - Vukasin Milovanovic (https://github.com/vuule)

URL: #11543
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request good first issue Good for newcomers improvement Improvement / enhancement to an existing function
Projects
None yet
Development

No branches or pull requests

6 participants