refactor(tools): organize post_experiment scripts; total vs query CPU; latency units - #742
Merged
Merged
Conversation
Split post_experiment/ into single_experiment/ (per-experiment cost, latency, fidelity, throughput analysis), multi_experiment/ (cross- experiment comparison plots), lib/ (results_loader) and debug/. Replace plot_scale_vs_metrics.py with its v2 (adds --experiment_mode) and track plot_latency_cost_tradeoff.py, plot_scale_vs_benefits.py and plot_cardinality_vs_benefit_v2.py. Fix sys.path, imports and sibling script paths for the new layout; update doc references. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… query CPU Replace regex parsing of compare_costs.py / run_compare_latencies.sh text output in the multi_experiment scripts with --machine-readable JSON. plot_scale_vs_metrics' default cost is now total CPU p95 (it previously never matched for baseline and matched only the query engine for sketchdb). Add --benefit-type total_cost to the cardinality plots and name total vs query CPU in all cost labels. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Query latency is recorded as time.time() differences (seconds) and is never converted, but plot_latency_cost_tradeoff.py labeled it [ms]. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… values - plot_latency_cost_tradeoff: only require the CPU stats for the chosen --cpu_type; drop unsupported --cost_metric mean. - plot_scale_vs_metrics: return None with a warning when compare_costs omits query CPU instead of raising KeyError. - plot_cardinality_vs_benefit(_v2): drop non-finite benefit ratios before plotting; inf made the y-tick loop unbounded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…infinite values" This reverts commit 34be6a7.
Every caller already passes these arguments; removing the defaults makes a forgotten argument (e.g. experiment_mode, metric, benefit_type, total) an error instead of silently selecting baseline/p95/query CPU. Inline verify_scale (always True), analyze_throughput's label_filter (always the float-only filter) and calculate_stable_throughput's num_windows (always self.num_windows). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed and why
Organized the post-experiment scripts by what they analyze.
post_experiment/was a flat directory of ~20 scripts, which made it hard to tell per-experiment analysis apart from cross-experiment figure generation. It is now split into:single_experiment/: analyzes one experiment (cost, latency, fidelity, throughput)multi_experiment/: sweeps many experiments to produce comparison plotslib/: shared loadersdebug/: inspection toolsImports, script paths and doc references were updated to match, and a short README explains the layout.
Made each cost number say which cost it is.
Figures could mix up two different CPU costs:
Cost labels now say "Total CPU" or "Query CPU", and the cardinality plots gain a
total_costoption.Fixed cost values that were silently wrong or missing.
The multi-experiment scripts used regexes to scrape numbers from the text output of other scripts. The scale scripts' default "cost p95" never matched for Prometheus (returned nothing), and for ASAPQuery matched only the query engine, leaving out Prometheus's own CPU. These scripts now read the JSON output of
compare_costs.py/compare_latencies.py, so a format change raises an error instead of quietly dropping data. The default cost is now total CPU p95 for both systems.Fixed latency units on the latency–cost tradeoff plot.
Latency is measured in seconds end to end, but the plot labeled it milliseconds. Absolute values were therefore shown 1000× too small; ratios were unaffected. Labels now say seconds.
Removed default argument values that could hide mistakes.
Many functions defaulted to baseline mode, p95 or query CPU even though every caller passes these values explicitly. A future call that forgot one would silently get the wrong numbers; it now raises an error. Parameters that were never varied were folded into the function body.
Kept one version of
plot_scale_vs_metrics.py, the newer one that adds an--experiment_modeoption. Also added the previously untracked plotting scripts to git.Figures to regenerate
plot_scale_vs_metrics.py/plot_scale_vs_benefits.pyusing the default cost: it is now total CPU, where before it was missing or engine-only.How it was checked
🤖 Generated with Claude Code