Compare persistent pipeline and separate stage execution - #55
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: NVIDIA/cuPhoton/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe pull request adds an XScan benchmark that compares a persistent device pipeline with separately launched, file-mediated stages. It adds synthetic fixture generation, output parity checks, timing and provenance reports, a command-line interface, documentation, and tests. ChangesPipeline Stage Benchmark
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to No actionable issue is established in the supplied changes; the benchmark command is mergeable after normal checks. 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Comment |
melo-gonzo
left a comment
melo-gonzo
left a comment
There was a problem hiding this comment.
Approving with three should-fix items on fairness; the design itself is sound and clearly labeled. Both arms run in real child processes, so the staged arm pays interpreter start, imports, CuPy and torch initialization, and model load per stage as a CLI chain would, while the pipeline arm initializes one DeviceWorkerContext and loops. Inputs, options, checkpoint hash, dtype, and variance policy are identical, warmup rounds are reported separately, device clocks are synchronized before stopping, and every round is checked for exact equality of 22 arrays plus candidate identity, diagnostics, feature names, and predictions before any timing is compared. This is the right shape for the question the Rubin perf audit raised, where GPU work was 15 to 25 s of a 377 s chain. Three things make the headline ratio overstate the staged penalty or limit how citeable the report is; inline notes at each. CPU tests cover comparison, child failure and timeout, round bookkeeping, tamper rejection, and CLI parsing; run_stage, audit, prepare_fixture, and an end-to-end compare are CPU-runnable and untested, and should be added. All four ci-required entries are green.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Add a reproducible matched-API benchmark with explicit startup and warm batch timing, file-mediated stage boundaries, and scientific parity checks for every image and round. Signed-off-by: Trent Nelson <trentn@nvidia.com>
Signed-off-by: Trent Nelson <trentn@nvidia.com>
Signed-off-by: Trent Nelson <trentn@nvidia.com>
|
@coderabbitai review |
5cf9945 to
c808753
Compare
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
2 participants
Running xPOIS, xFit and XScan as separate processes incurs startup, transfer and intermediate-file costs that the persistent device pipeline can amortize.
cuphoton xscan benchmark-pipelinecompares those workflows using identical numerical APIs, inputs, candidate batches and checkpoint. The staged baseline uses real process/file boundaries; it does not reproduce the stock component CLI chain, which changes some arithmetic paths.Both routes retain attempted-round evidence and compare all 22 compact scientific arrays, candidate identities and predictions before reporting success. The staged route writes only arrays consumed downstream or required for parity. Raw timings retain all verification; the headline ratio subtracts measured intermediate hashing and repeated xFit/XScan input rechecks. The original-input hashes common to both routes remain included. This subtraction cannot undo verification's cache effects. Report provenance identifies the selected GPU, driver, CUDA runtime and CuPy distribution.
Validation: 54 CPU benchmark and CLI-contract tests passed after local executor integration, including fixture generation and both comparison orders through stage summaries and parity audits. Repository lint and all pre-commit checks passed. GPU qualification was performed on 5cf9945f, before executor integration. On one RTX PRO 6000 Blackwell (driver 610.57.04, CUDA runtime 13.2, CuPy 14.1.1), four 1024×1024 pairs with nine candidates each and 17×17 stamps passed exact parity in both launch orders using the same fixture. Each order retained one warmup and three measured rounds: 32 item-round checks and 704 array comparisons total. Warm pipeline medians were 0.884/0.901 seconds; raw fresh-stage medians were 8.211/8.407 seconds; fresh-stage medians with additional hashing excluded were 8.100/8.293 seconds. Setup and whole-invocation timings remain separately available in the reports. These measurements use the updated benchmark; earlier results with additional outputs are superseded. Integration preserves the benchmark code and the numerical functions it calls. It also inherits changes elsewhere in training.py, so the retained GPU source manifest does not describe the complete integrated revision; the GPU benchmark was not rerun after integration.
These are synthetic workflow timings with an untrained fusion model, not classifier accuracy, isolated kernel speedup or multi-GPU performance claims. This PR is rebased on the merged executor stack and uses the existing command/invariant framework.