Store jsonbin sidecar paths relative to JSON output file location#405
Conversation
Keep --jsonbin sidecar files written next to the JSON output file, but record their filenames relative to the JSON file directory. This makes generated JSON and its sidecar directories relocatable as a unit and avoids embedding the benchmark process working-directory relationship into the JSON payload. Add a focused json_printer test that verifies sample-time and sample-frequency sidecars are written beside the JSON file while summary filename fields contain JSON-relative paths.
|
@coderabbitai full review |
✅ Action performedFull review finished. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR separates binary sidecar write paths from JSON-relative filename metadata, updates both sidecar-producing branches to use the shared helper, and adds tests plus build registration covering success and failure cases. ChangesJSON-relative sidecar path handling
Assessment against linked issues
No out-of-scope changes found. Comment |
Extract the duplicated sample-times and sample-frequencies jsonbin sidecar write paths into a shared helper. This keeps directory creation, file writing, warning logging, summary metadata, and write-duration logging in one place for both sidecar types.
Cover the case where the expected sidecar directory path collides with a regular file. Verify that json_printer keeps descriptive summary metadata but omits filename and size when the sidecar file was not written. Register printer and verify output.
Factor repeated optional benchmark-printer logging in write_jsonbin_sidecar into a local helper used by both warning and success paths. Also tidy the jsonbin summary names for sample time and frequency sidecar files.
|
@coderabbitai full review |
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e6bdac4b-ae58-48b5-a928-55cc842fec5e
📒 Files selected for processing (3)
nvbench/json_printer.cutesting/CMakeLists.txttesting/json_printer.cu
|
The work was motivated by failure to ingest JSON file and sidecar binary files when using #386 while JSON files were generated by running CCCL benchmarks from devcontainer. Commands used to reproduce./bin/cub.bench.bitonic_sort.warp_keys.base -d 1 --cold-warmup-runs 16 --min-samples 100 --stopping-criterion entropy --min-r2 0.88 --jsonbin ../../../perf_data/warp-bitonic-sort-run1.json
./bin/cub.bench.bitonic_sort.warp_keys.base -d 1 --cold-warmup-runs 16 --min-samples 100 --stopping-criterion entropy --min-r2 0.88 --jsonbin ../../../perf_data/warp-bitonic-sort-run2.json
PYTHONPATH=./_deps/nvbench-src/python/scripts python ./_deps/nvbench-src/python/scripts/nvbench_compare.py ../../../perf_data/warp-bitonic-sort-run1.json ../../../perf_data/warp-bitonic-sort-run2.json |
Closes #404
Keep --jsonbin sidecar files written next to the JSON output file, but record their filenames relative to the JSON file directory. This makes generated JSON and its sidecar directories relocatable as a unit and avoids embedding the benchmark process working-directory relationship into the JSON payload.
Add a focused json_printer test that verifies sample-time and sample-frequency sidecars are written beside the JSON file while summary filename fields contain JSON-relative paths.