Update cudf-polars benchmarks for new default engine - #22619
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Benchmark section of the cudf-polars docs was rewritten to present a GPU-engine scaling overview, embed TPC-DS benchmark tables for SF1k and SF3k (PDS-H and PDS-DS), and update the concluding link to the PDS-DS queries in the cuDF repository. ChangesBenchmark Documentation Update
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/cudf/source/cudf_polars/index.md`:
- Around line 55-67: The section mixes benchmark names (“Polars Decision Support
(PDS)”, “PDS-DS”, and figure filenames referencing “tpcds”); update all
occurrences to a single consistent term (use “TPC-DS”)—replace the phrase
"Polars Decision Support (PDS)" with "Polars TPC-DS benchmark" (or similar),
change any "PDS-DS" tokens to "TPC-DS", and ensure figure captions/alt text and
surrounding sentences reference "TPC-DS" consistently (including references to
polars_tpcds_sf1k.png and polars_tpcds_sf3k.png).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 83387283-7a4a-415c-8a63-1adebad9f57a
⛔ Files ignored due to path filters (2)
docs/cudf/source/_static/polars_tpcds_sf1k.pngis excluded by!**/*.pngdocs/cudf/source/_static/polars_tpcds_sf3k.pngis excluded by!**/*.png
📒 Files selected for processing (1)
docs/cudf/source/cudf_polars/index.md
| We ran the Polars Decision Support (PDS) benchmarks to compare the Polars GPU engine with the CPU engine at larger scale factors to show how the GPU engine delivers meaningful speedups as dataset size grows: | ||
|
|
||
| ```{figure} ../_static/pds_benchmark_polars.png | ||
| ```{figure} ../_static/polars_tpcds_sf1k.png | ||
| :width: 600px | ||
| ``` | ||
|
|
||
| You can see up to 13x speedup using the GPU engine on the compute-heavy PDS queries involving | ||
| complex aggregation and join operations. Below are the speedups for the top performing queries: | ||
| On a single GPU, you can run TB-scale workloads with significant speedups compared to running on CPU. You can also scale up to run on multiple GPUs for processing even larger workloads: | ||
|
|
||
| ```{figure} ../_static/compute_heavy_queries_polars.png | ||
| ```{figure} ../_static/polars_tpcds_sf3k.png | ||
| :width: 1000px | ||
| ``` | ||
|
|
||
| *PDS-H benchmark | GPU: NVIDIA H100 PCIe | CPU: Intel Xeon W9-3495X (Sapphire Rapids) | Storage: | ||
| Local NVMe* | ||
|
|
||
| You can reproduce the results by visiting the [Polars Decision Support (PDS) GitHub repository](https://github.com/pola-rs/polars-benchmark). | ||
| For more information on the benchmarks being run, see the PDS-DS queries in the [cuDF GitHub repository](https://github.com/rapidsai/cudf/tree/main/python/cudf_polars/cudf_polars/streaming/benchmarks). |
There was a problem hiding this comment.
Unify benchmark naming across the section.
The text mixes “PDS”, “TPC-DS” (in figure names), and “PDS-DS”. Please standardize to a single term (likely TPC-DS unless your internal suite is intentionally named otherwise) to avoid reader confusion.
As per coding guidelines, “Consistency: Version numbers, parameter types, and terminology match code.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/cudf/source/cudf_polars/index.md` around lines 55 - 67, The section
mixes benchmark names (“Polars Decision Support (PDS)”, “PDS-DS”, and figure
filenames referencing “tpcds”); update all occurrences to a single consistent
term (use “TPC-DS”)—replace the phrase "Polars Decision Support (PDS)" with
"Polars TPC-DS benchmark" (or similar), change any "PDS-DS" tokens to "TPC-DS",
and ensure figure captions/alt text and surrounding sentences reference "TPC-DS"
consistently (including references to polars_tpcds_sf1k.png and
polars_tpcds_sf3k.png).
| Local NVMe* | ||
|
|
||
| You can reproduce the results by visiting the [Polars Decision Support (PDS) GitHub repository](https://github.com/pola-rs/polars-benchmark). | ||
| For more information on the benchmarks being run, see the PDS-DS queries in the [cuDF GitHub repository](https://github.com/rapidsai/cudf/tree/main/python/cudf_polars/cudf_polars/streaming/benchmarks). |
There was a problem hiding this comment.
This should point at the release branch, not main. Also, this doesn't give the reader enough information to reproduce, because there are a bunch of non-default parameters (hopefully fewer than there used to be) being passed.
There was a problem hiding this comment.
Ideally if Polars adds TPC-DS queries to their polars-benchmark repo we should point there (like we did previously for TPC-H). Today our benchmarking is not easily reproducible due to the data generation process. Is it better to link to the queries for some context or just drop this altogether?
There was a problem hiding this comment.
I think it's ok to be imperfect. When we write up the point-in-time benchmark statement we should have an audit trail though.
I was just wondering, since this goes in both "release" and "nightly" docs if it makes sense to always link to what is equivalent to "nightly" code.
|
Actionable comments posted: 0 |
|
Actionable comments posted: 0 |
|
Squash-merging this PR for 26.06 during code freeze since we want these numbers in the released docs for 26.06. |
4 participants
Description
Replace benchmarks in the cudf-polars docs with updated PDS-DS results collected using the updated GPU engine with rapidsmpf.
Closes #21471
Checklist