Skip to content

Capture command line on pds-h benchmark runner #22508

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

Merged
rapids-bot merged 7 commits into from
May 18, 2026
Merged

Capture command line on pds-h benchmark runner #22508

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ddb48c9
Capture command line on pds-h benchmark runner
TomAugspurger May 14, 2026
73cdaca
Capture env vars too
TomAugspurger May 14, 2026
f29fd19
Update python/cudf_polars/cudf_polars/experimental/benchmarks/utils_n…
TomAugspurger May 15, 2026
4170f90
Merge branch 'main' into tom/capture-command
TomAugspurger May 15, 2026
21cf3b1
Merge remote-tracking branch 'upstream/main' into tom/capture-command
TomAugspurger May 18, 2026
a5f7ece
lint
TomAugspurger May 18, 2026
1ec67db
Merge branch 'main' into tom/capture-command
TomAugspurger May 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions python/cudf_polars/cudf_polars/experimental/benchmarks/utils_new_frontends.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import logging
import os
import pprint
import shlex
import sys
import textwrap
import time
Expand Down Expand Up @@ -430,6 +431,8 @@ class RunConfig:
timestamp: str = dataclasses.field(
default_factory=lambda: datetime.now(UTC).isoformat()
)
command_line: str
capture_env_vars: str
Comment on lines +434 to +435

@coderabbitai coderabbitai Bot May 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid breaking RunConfig constructor compatibility.

Adding required command_line and capture_env_vars fields to an exported dataclass introduces a breaking API change for any direct RunConfig(...) callers. Please give these fields safe defaults (e.g., empty strings) or add a compatibility path.

As per coding guidelines: python/**/*.{py,pyx}: Detect and flag API breaking changes to public methods/attributes without deprecation warnings.

🤖 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
`@python/cudf_polars/cudf_polars/experimental/benchmarks/utils_new_frontends.py`
around lines 434 - 435, RunConfig's dataclass now requires command_line and
capture_env_vars which breaks callers; make these fields optional with safe
defaults (e.g., empty string) so the RunConfig constructor remains compatible.
Update the RunConfig dataclass declaration (the fields named command_line and
capture_env_vars) to provide default values (or mark them Optional with
defaults) so existing direct RunConfig(...) invocations continue to work without
changes.


def __post_init__(self) -> None: # noqa: D105
if self.io_mode == "hot" and self.iterations < 2:
Expand All @@ -438,6 +441,12 @@ def __post_init__(self) -> None: # noqa: D105
"iteration 0 warms the cache, iterations 1+ are the hot measurements."
)

# Update `extra_info.environment` with the captured environment variables.
self.extra_info.setdefault("environment", {})
for var in self.capture_env_vars.split(","):
var_ = var.strip()
self.extra_info["environment"][var_] = os.environ.get(var_)

Comment thread
TomAugspurger marked this conversation as resolved.
@classmethod
def from_args(cls, args: argparse.Namespace) -> RunConfig:
"""Create a RunConfig from command line arguments."""
Expand Down Expand Up @@ -543,6 +552,8 @@ def from_args(cls, args: argparse.Namespace) -> RunConfig:
duckdb_threads=args.duckdb_threads,
duckdb_memory_limit=args.duckdb_memory_limit,
duckdb_temp_dir=args.duckdb_temp_dir,
command_line=shlex.join(sys.argv),
Comment thread
TomAugspurger marked this conversation as resolved.
Comment thread
mroeschke marked this conversation as resolved.
capture_env_vars=args.capture_env_vars,
)

def serialize(self, engine: StreamingEngine | None) -> dict:
Expand All @@ -567,6 +578,7 @@ def serialize(self, engine: StreamingEngine | None) -> dict:
"extra_info": self.extra_info,
"run_id": str(self.run_id),
"timestamp": self.timestamp,
"command_line": self.command_line,
"streaming_options": {
"rapidsmpf": opts.to_rapidsmpf_options().get_strings(),
"executor": opts.to_executor_options(),
Expand Down Expand Up @@ -1892,6 +1904,12 @@ def build_parser(num_queries: int = 22) -> argparse.ArgumentParser:
default=None,
help="Directory for DuckDB to spill temporary data to disk.",
)
parser.add_argument(
"--capture-env-vars",
type=str,
default="CUDF_POLARS_LOG_TRACES_MEMORY,CUDF_POLARS_LOG_TRACES,DASK_DISTRIBUTED__COMM__TIMEOUTS__CONNECT,DASK_DISTRIBUTED__COMM__UCX__CONNECT_TIMEOUT,KVIKIO_NTHREADS,LIBCUDF_NUM_HOST_WORKERS,OMP_NUM_THREADS,POLARS_MAX_THREADS,RAPIDSMPF_num_streaming_threads,UCX_MAX_RNDV_RAILS,UCX_PROTO_ENABLE,UCX_RNDV_FRAG_MEM_TYPES,UCX_RNDV_MTYPE_WORKER_FC_ENABLE,UCX_RNDV_MTYPE_WORKER_MAX_MEM,UCX_RNDV_PIPELINE_ERROR_HANDLING",
Comment thread
Matt711 marked this conversation as resolved.
help="Comma-separated list of environment variables to capture. Written to ``extra_info.environment``.",
)

StreamingOptions._add_cli_args(parser)

Expand Down
Loading