IBM QAOA: circuit-preparation cost model for Fig. 12, depth-7 campaigns, dead-code pass - #88
anurag-r20 wants to merge 43 commits into
Conversation
Step 1 of the IBM_QAOA cleanup plan (Assess -> Protect -> Restructure, per sefop-training-hub's refactoring guide): lock current behavior with tests before moving anything, so the eventual move into src/utils.py can be verified against these rather than against assumed behavior. Covers the 6 of 9 notebook-inline helpers that are already self-contained (no hidden dependency on notebook-level globals): _relativize_warning_filename, _scrubbed_formatwarning, _relativize_paths, _rescale_resource, _sim_entries, _label_hw_frontier. Each is extracted verbatim from the notebook's own JSON at collection time (throwaway scaffolding -- goes away once Step 2 moves these into src/utils.py and this file is repointed to import them normally). Remaining 3 (_best_bitstring_ar, _build_hw_frontier, _sim_winners) depend on notebook globals (color_map, minmax paths, cached instance contexts) rather than parameters, so they need a small signature fix before they're testable in isolation -- tracked as the next step, not done here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tep 1/5, complete)
_best_bitstring_ar, _build_hw_frontier, and _sim_winners (cell 612c9536)
had a minimal, behavior-preserving signature change first: they relied on
notebook-level globals (_bb_minmax_path, df_hardware_bb, color_map, etc.)
instead of explicit parameters, which made them untestable in isolation.
Added the missing parameters, updated the three call sites to pass the
same values explicitly, and wrote characterization tests against real
(non-mocked) helper functions from src.utils/src.approx_ratio_calc:
- TestBestBitstringAr: hand-verified against a synthetic 3-node/2-edge
MaxCut instance (traces maxcut_energy_from_bitstring's bit-reversal and
cut-value logic), plus a minmax-cache-hit test.
- TestBuildHwFrontier: exercises the real prepare_ibm_qaoa_plot_data /
build_recommendation_data pipeline end to end, plus a missing-column
KeyError case.
- TestSimWinners: exercises the real _pareto_envelope_and_owner with a
dominating-entry case, a crossover case, and a too-few-points case.
Also fixed _extract_function's end-of-function detection: it looked for
the next unindented line to know where a function body ends, which broke
on _best_bitstring_ar's multi-line def signature (the closing `) -> ...:`
line is itself unindented). Now tracks paren balance across the signature
before applying that check.
612c9536's execution_count/outputs are cleared since the signature edit
hasn't been re-run yet -- full end-to-end re-execution happens at Step 3
(import consolidation) per the approved cleanup plan, not before.
All 9 notebook helpers now have characterization tests (28 total in this
file); 526/526 in the full suite pass; flake8 (F401,F841,E9,F63,F7,F82)
clean on the touched files. Completes Step 1 ("Protect") of the IBM_QAOA
cleanup plan.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ete)
_relativize_warning_filename, _scrubbed_formatwarning, _relativize_paths,
_rescale_resource, _best_bitstring_ar, _build_hw_frontier, _sim_entries,
_label_hw_frontier, and _sim_winners now live in src/utils.py instead of
as inline defs scattered across notebook cells 914a9b2d, d78417eb,
7f87d664, and 612c9536.
_relativize_warning_filename/_scrubbed_formatwarning gained an explicit
workspace_root parameter (previously a closure over the notebook's
WORKSPACE_ROOT global); since warnings.formatwarning is called
positionally by the warnings module, the notebook now binds it via
functools.partial(_scrubbed_formatwarning, workspace_root=WORKSPACE_ROOT)
rather than assigning the bare function. The other 7 functions moved
verbatim (their signatures were already made explicit in the prior
commit).
Each notebook cell now imports its helper from cell f21120d5's existing
`from src.utils import (...)` block instead of defining it locally; the
now-dead imports/aliases each removed def left behind (_get_minmax,
_extract_minmax_args, _maxcut_approximation_ratio,
_prepare_ibm_qaoa_plot_data, _build_recommendation_data,
_curve_from_response_summary, _method_label_from_training_method in cell
612c9536) were removed too, verified by checking every remaining call
site of each name individually rather than assuming.
tests/test_ibm_qaoa_notebook_helpers.py now imports all 9 straight from
src.utils instead of exec'ing their source out of the notebook's JSON;
test bodies are unchanged (still call through a small per-fixture `ns`
dict) so the diff from the pre-move version stays minimal and the tests
verify the move byte-for-byte. _best_bitstring_ar's tests patch
src.utils._get_minmax/_extract_minmax_args directly instead of injecting
replacements as call-site parameters, since those are now module-level
names inside src.utils rather than caller-supplied.
d78417eb and 7f87d664's stale outputs (a dataframe preview, a scale-factor
printout) are cleared since their source changed; full end-to-end
re-execution happens at Step 3 (import consolidation), not before.
28/28 in this test file pass against the moved functions; 526/526 in the
full suite pass; flake8 (F401,F841,E9,F63,F7,F82) clean on
src/utils.py and the touched test file. Completes Step 2 ("Restructure")
of the IBM_QAOA cleanup plan.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Cell 612c9536 still had its own from src.Processing import (...), from src.approx_ratio_calc import (...), and from src.utils import (is_empty_nested_list as ...) blocks, plus a redundant import importlib/import matplotlib.patheffects; cell 914a9b2d still had its own import warnings as _warnings_module. All moved into cell f21120d5's existing import block (warnings kept its _warnings_module alias so the one call site referencing it didn't need to change). Left alone, on purpose: the importlib.reload(module)-then-refetch blocks in 914a9b2d (simulation_validation), 5c3c9452 and b66e6da6 (plot_multi_method_window_sticker_component_panels / plot_pareto_frontier_overlay), and 612c9536 (the six _pareto_envelope_and_owner/_build_family_color_map/etc. names). These aren't leftover scattered imports, they're a deliberate freshness pattern for functions actively tweaked all session so a single cell re-run picks up src/ edits without re-running cell 1 (which %autoreload 2 doesn't reliably do for these). Folding them into cell 1's static imports would remove that. 526/526 full suite passes (untouched by this change); flake8 clean on the touched Python files; cell 1's source compiles and its imports resolve when exec'd standalone. Full end-to-end notebook re-execution is still pending (the user is running it locally) before Step 4. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
You said you'll run the import cell first anyway, so the freshness tradeoff those blocks existed for no longer applies. Removed all 4 reload(module)-then-refetch blocks and added the 8 names they were re-fetching as plain imports in cell f21120d5: - 914a9b2d: estimate_hardware_time_per_shot (was already imported directly in cell 1 too, so this block was pure redundancy) - 5c3c9452: plot_multi_method_window_sticker_component_panels - b66e6da6: plot_pareto_frontier_overlay - 612c9536: _pareto_envelope_and_owner, _pareto_envelope_bounds, _build_family_color_map, _draw_family_colorbars, _family_colorbar_row_count, _label_depth Also dropped cell 1's own `import importlib`, now unused anywhere in the notebook since no importlib.reload(...) calls remain. 526/526 full suite still passes (unaffected); cell 1's source compiles and its imports resolve when exec'd standalone; every other cell is byte-identical. 5c3c9452/b66e6da6's stale plot-image outputs are cleared since their source changed. Every notebook import now lives in cell 1, so from here on a full re-run starts there. Full end-to-end re-execution is still pending (the user is running it locally) before Step 4. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Verifies the Step 1-3 IBM_QAOA cleanup (characterization tests, moving all 9 helpers into src/utils.py, consolidating every import into cell 1) against a real run: execution counts 1->14 monotonic, zero error outputs across all 14 code cells, and no source changed (outputs/execution_count only). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Confirmed-dead, zero callers anywhere in the tree (re-verified fresh against the current tree, not the earlier line-number snapshot): run_strategy_with_final_sampling, fit_exact_resource_models, and its three private helpers _fit_exact_resource_model, _predict_exact_resource_model, _exact_resource_model_metrics -- all in simulation_validation.py. Also caught one the earlier assessment missed: _positive_numeric_frame, which turns out to be called only from within the now-dead fit_exact_resource_models, so it was already transitively dead. 316 lines removed. The 6 "imported but never called" borderline candidates from the assessment (plot_training_bricks, title_from_instance_names, make_asof_per_file in Analysis.ipynb; plot_method_curves, plot_multi_method_window_sticker_components, build_binned_budget_dataset in Simulation_Method_Validation_and_WS.ipynb) are handled per the plan's own caution: dropped the dead import lines, left the function bodies in src/ alone for now, since "imported nowhere is called" is weaker evidence than "zero references at all" and plot_training_bricks in particular looks superseded by plot_ibm_qaoa_training_bricks (which IS imported and called in Analysis.ipynb) rather than confirmed dead. 526/526 full suite passes; flake8 (F401,F841,E9,F63,F7,F82) clean on simulation_validation.py and utils.py; both touched notebook cells' imports compile and resolve standalone; every other cell in both notebooks is byte-identical (verified cell-by-cell, not just via diff stat). Both notebooks' import cells need a re-run before their execution counts/outputs reflect this change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ep 5)
get_minmax, extract_minmax_args, and best_prefix_metrics had no test
coverage at all before this (load_maxcut_instance_context,
counts_from_bitstring_samples, maxcut_energy_from_bitstring, and
maxcut_approximation_ratio were already covered by
test_ibm_qaoa_processing.py). New file
tests/test_ibm_qaoa_approx_ratio_calc.py, following Gurobi_QP's
conventions (test__<function>__given_<condition>__<expected> naming,
ARRANGE/ACT/ASSERT blocks, one class per function).
get_minmax: one test per graph_type's filename pattern (heavy_hex,
erdos_renyi, line_to_full, random_regular), plus unknown-graph-type,
no-match, and multiple-match error paths.
best_prefix_metrics: uses the same 3-node/2-edge synthetic MaxCut
instance as TestBestBitstringAr in test_ibm_qaoa_notebook_helpers.py so
approximation ratios are hand-verifiable. Covers empty input, the
running-best-persists-until-beaten behavior across checkpoints (the main
subtlety: a worse bitstring sampled between two checkpoints must not
overwrite the still-better earlier best), checkpoint clipping beyond the
stream length, non-positive checkpoints being dropped, and duplicate
checkpoints collapsing to one row.
14/14 new tests pass; 540/540 full suite passes; flake8 clean. First
chunk of Step 5 ("Extend": broader test coverage) -- Processing.py
(QAOAHardware, and the resolve_hardware_training_costs pipeline's 8
untested private helpers) is next.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ing_costs (Step 5) None of this had any coverage before: it's the code behind the notebook's _build_hw_frontier / _best_bitstring_ar pipeline that the hardware-overlay Pareto plot from this whole session depends on. New file tests/test_ibm_qaoa_hardware_training_costs.py (40 tests), Gurobi_QP conventions throughout. QAOAHardware: locate_hardware_instance's heavy_hex glob pattern plus unknown-graph-type; load_hardware_instance's job_p/training_p regex extraction (including the fallback from result_file's suffix when the method string has no trailing digit), per-record filtering (missing eval_energy, empty counts), and QPU_time being split evenly across however many valid circuit records a job-level file contains. The 8-function resolve_hardware_training_costs pipeline: each of _normalize_angle_list, _max_abs_angle_diff, _as_finite_float, _choose_stage_for_method (all 3 stage-selection branches: duplicate-resolution, no_opt-picks-min, opt-picks-max, plus the ambiguous-returns-None case), _build_stage_manifest (stage ordering, missing pre_processing_time, a stage missing train_duration being skipped without disturbing the running total), _build_inner_duration_tables (cumulative duration within an iteration), _resolve_training_stage (the most branchy one: missing training file, missing params, exact angle match, no match without depth-prefix fallback, the depth-prefix fallback itself when training_p > job_p, and the duplicate-physical-file note), and _resolve_inner_duration (previous-stage total plus current-stage total gated on depth_step <= job_p). Plus two integration-style tests on the public resolve_hardware_training_costs entry point itself. 40/40 new tests pass; 580/580 full suite passes; flake8 clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Prioritized subset per plan: pure data-shaping functions, not the matplotlib-heavy plot_* functions (verified visually all session, not a good unit-test target). New file tests/test_ibm_qaoa_utils_data_prep.py, 46 tests covering: is_empty_nested_list, sem, counts_to_samples_df, curve_from_training_summary, curve_from_window_summary, cross_strategy_envelope, _is_no_opt_metadata, _force_dagger_label, curve_label, prepare_monotone_curve, _pareto_envelope_bounds, concat_summary, attach_result_metadata, shared_approx_ylim, shared_approx_yticks, _percent_approx_ylabel, _percent_axis_values, and prepare_training_bricks_data. prepare_training_bricks_data gets the most scrutiny (4 tests): its own PR history records a real misalignment bug fixed earlier in this engagement (a positional `.sem().values` assignment replaced with an explicit merge on ["job_p", "method_base"]), and it had zero test coverage until now despite that. One test is a structural regression guard for that class of bug (asserts each (job_p, method_base) group's sem_total is reachable only via the merge key, not row position); another confirms step_* columns are correctly zeroed past a row's own job_p depth. 626/626 full suite passes; flake8 clean. This closes out the "prioritized subset" of utils.py coverage agreed for this chunk; remaining untested utils.py surface (I/O-touching summary loaders, window-sticker style/color-map helpers, and the plot_* functions themselves) is left for a future pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… (Step 5, final chunk)
Second and final chunk of utils.py coverage, completing Step 5 of the
IBM_QAOA cleanup plan. New file tests/test_ibm_qaoa_utils_style_and_io.py,
36 tests covering: _window_sticker_label_base, _window_sticker_label_depth,
_shade_color, _lighten_color, window_sticker_method_color (one test per
paper-color family), window_sticker_method_color_map (including the
same-family depth-shading behavior), _ws_display_method_label,
_ensure_save_dir, _display_cross_strategy_envelope,
_prepare_parameter_curve, and the result-root/summary-CSV loaders
(resolve_result_root, read_summary_csv, read_first_summary_csv,
rebuild_strategy_budget_summary, load_multi_strategy_summaries).
Deliberately out of scope, noted in the file's own docstring: the QPS
method-label/color resolution chain (_compact_method_label,
_plain_method_label_from_training_method, _method_color_from_training_method,
_window_sticker_method_color, _marker_from_training_method,
_style_plot_kwargs, and several smaller helpers feeding them) -- lower-
value style plumbing already implicitly exercised via
_method_label_from_training_method's existing coverage, and the 6
functions with dead imports dropped in the Step 4 commit
(title_from_instance_names, make_asof_per_file, plot_training_bricks,
plot_method_curves, plot_multi_method_window_sticker_components,
build_binned_budget_dataset) -- not worth testing code nothing calls.
Also unchanged from prior chunks: the matplotlib-heavy plot_* functions
themselves stay out of scope, verified visually against rendered PNGs all
session rather than unit tested.
662/662 full suite passes; flake8 clean.
This completes Step 5 ("Extend") and the full 5-step IBM_QAOA cleanup
plan: Protect (28 characterization tests for the notebook-inline
helpers), Restructure (moved into src/utils.py, imports consolidated
into cell 1), Restructure/dead-code (316 lines + 6 dead imports removed),
and Extend (136 new tests across approx_ratio_calc.py, Processing.py, and
utils.py's data-prep/style/IO helpers).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tep 5, continued) Extends Step 5 into simulation_validation.py, the biggest and least-tested file in IBM_QAOA/ (105 top-level defs, previously only 5 had direct tests plus 3 covered indirectly from test_ibm_qaoa_processing.py). New file tests/test_ibm_qaoa_simulation_validation_data_logic.py, 93 tests across 41 functions/properties. Prioritized the same way as the utils.py/Processing.py passes: pure, deterministic logic only. Explicitly out of scope (documented in the file's own docstring): qiskit/Aer circuit-building and sampling (build_bound_qaoa_circuit, MPSAerSampleEvaluator, sample_fixed_angles, ...), external-pipeline-repo-dependent functions (generate_linear_ramp_angles, run_method_from_config, ensure_pipeline_imports, ...), the multi-hundred-line orchestration functions (run_pt_pss_exact_points, run_fa_pss_exact_points, generate_pss_exact_points, run_stochastic_benchmark_pss, setup_stochastic_benchmark_campaign, build_test_instance_set_from_repo, ...), and build_binned_budget_dataset (dead code since Step 4). Particular attention to _build_response_summary_from_rec_params: this is the actual function defining the response_lower/response_upper = response +/- 1.96*SEM convention this whole engagement's plot-styling work depended on (referenced from memory repeatedly this session, never directly verified until now) -- one test locks that computation down from first principles, another confirms a non-zero native CI column overrides the computed one. Also covered: snap_actionable_fit_to_feasible_grid (the function whose fit algorithm was changed earlier in this engagement, per the PR body, and was untested until now), the budget-grid builders (build_dense_budget_grid, build_budget_bin_edges, _centers_from_edges/ _edges_from_centers), the exact-point completeness/dedup logic (_exact_group_is_complete, _deduplicate_exact_points), and build_sampled_training_config's branching (TransferTrainer, FixedAngleConjecture/TQATrainer, RecursionTrainer's nested trainer_init, cobyla_maxiter application). Found while writing the assign_deterministic_train_split test (not fixed, just noted): the function str-casts interp_results' join key (merged[instance_col].astype(str)) but not exp_raw_df's, so a numeric exp_raw_df["instance"] column raises a dtype-mismatch merge error instead of joining. Not observed to occur in this codebase's real dataframes (instance columns are consistently zero-padded strings elsewhere, e.g. InstanceSpec.instance_id), so left as-is with a comment rather than changed speculatively. 93/93 new tests pass; 755/755 full suite passes; flake8 clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Verifies the fully-committed IBM_QAOA cleanup (all 5 steps) against a fresh run: execution counts 1->14 monotonic, zero error outputs across all 14 code cells, no source changed (outputs/execution_count only). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nel figure
Hardware measurement (144-node heavy-hex MaxCut, depth 1-5, M in 10..10000,
three runs averaged): one submitted circuit takes 13.87 s to return and that
barely moves with the shot count, because compilation, transfer, queueing and
control-electronics load dominate. The published proxy model bills shot time
only, so it understates the cheap end of Fig. 12 by two orders of magnitude:
its cheapest optimized point is N=10, M=10, Q=200 at 0.12 s, which really
costs ~16 submitted jobs.
No simulation is re-run anywhere. The (N, M, Q) -> approximation-ratio
mapping is physics and carries no dependence on how resources are priced, so
this re-costs the existing exact points and redoes only the cheap
post-processing (~31 s per campaign).
src/simulation_validation.py
- recost_exact_points_with_circuit_prep(): charges every submitted job
t_prep, including the final sampling job (which is what stops
zero-training strategies from looking free). The multiplier is the
recorded num_objective_evaluations, not N: COBYLA submits 15 circuits for
maxiter=10 and 114 for 100, so pricing per iteration would undercount by
10-50%. Optionally also bills the recorded training shots instead of N*M,
which the old model undercounted for the same reason.
- infer_proxy_time_per_shot(): recovers a campaign's calibrated shot rate
from its own stored costs, since campaigns differ slightly.
- build_pss_proxy_costs(): the clean proxy model lifted out of notebook
cell d58761a8 so the notebook and the new script share one
implementation rather than drifting. Verified to reproduce the previous
inline formula exactly across 10,290 real FA_PP_opt rows, and its PT
branch checked against a real PT campaign.
- pt_transfer_strategy_mask(): the alphanumeric-boundary matching that
keeps "FA_PP_opt" and "LR_opt" from being read as Parameter Transfer on
account of the "pt" inside "_opt".
src/utils.py
- _envelope_segment_bounds() / _draw_pareto_envelope_segments(): the
owner-coloured segment drawing that was inline in the Pareto cell, now
shared with the new figure.
- plot_cost_model_comparison_panels(): side-by-side panels with a shared
response axis and independent resource axes. Side by side rather than
stacked scales because the charge is n_evals * t_prep, which differs per
point, so no single axis rescaling can express it.
run_latency_recost.py
New script regenerating campaign roots under a given charge. It rebuilds
the baseline through the same path rather than reusing the committed CSVs,
because those date from August and predate the resource_match_bins change,
and the bootstrap is unseeded (#86) -- so the two panels have to be
generated together to differ by the charge alone.
Known limits, stated in the figure's own footnote: the charge makes shots
nearly free below M ~ 25,000 while the explored grid stops at M = 1000, so
the charged panel understates what a re-optimised grid would reach. Q grids
also differ by family (10,000 for optimized, 350,000 for zero-training);
--q-cap exists to check that sensitivity, since uncapped it flatters the
zero-training families.
797 tests pass (28 new); flake8 clean. The notebook's new cell needs the
script run first and is committed unexecuted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The families were not explored on the same sampling grid: Parameter Transfer and the zero-training Fixed Angles runs reach Q of a few hundred thousand, while every family that optimizes its angles stops at Q = 10,000. That gap is harmless while shots dominate the cost, but once a fixed per-submission charge compresses the cheap end it decides the whole picture. Uncapped, the charged frontier goes almost entirely to Parameter Transfer on sampling access rather than on merit, and Fixed Angles* never appears at all. Capped at a common Q <= 10,000 the optimized families reappear and the frontier separates into regimes, with zero-training methods holding it to ~160 s, Linear Ramp to ~550 s, and Fixed Angles* above that. Both variants are now notebook cells so the two can be run and compared before deciding which belongs in the paper. Interp is excluded from both (it never reaches the frontier and only stretches the resource axis, to 2.5e4 s once charged); one named list controls that. load_cost_model_panels() in src/utils.py assembles panels from the re-costed roots so each cell stays a handful of lines, raising when a variant has no roots at all and reporting rather than failing when only some are present. 802 tests pass (5 new); flake8 clean. Both cells executed standalone against the generated roots and produce their figures; committed unexecuted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…panels The first version of these panels dropped two things Fig. 12 carries: the Noiseless / Noise-Corrected calibration pair, and the measured ibm_boston frontier. Both are back, so each panel now shows the same three curve types as Fig. 12 and differs from its neighbour only by the preparation charge. The calibrations cannot be produced by rescaling one set of roots, because the charge is real wall-clock time while the shot terms scale with the shot rate. The basis is therefore fixed when the roots are generated: --shot-time-by-depth takes the noise-corrected map (t_noiseless / sqrt(gamma), sqrt(gamma) = F_CZ^(143 p), which is Table V), and --variant-tag keeps those roots from colliding with the noiseless ones. draw_hardware_frontier_steps() in src/utils.py is the hardware step curve lifted out of notebook cell 612c9536 so both figures share it. Its extra_cost charges the hardware workflow once rather than per objective evaluation: that workflow trained classically and submitted exactly one circuit, so one charge is all it incurs. That it barely moves under the charge, while the simulated curves move by orders of magnitude, is the point -- it already pays submission cost once. Hardware method labels now join the shared family colour map and the colorbar set, so a family keeps one colour across simulated and measured curves alike. load_cost_model_panels() takes the panel structure directly now, so a panel can carry several calibrations and a hardware overlay instead of exactly one curve. 807 tests pass (5 new, 6 rewritten for the new loader shape); flake8 clean. Rendering verified end to end with a synthetic hardware frontier; the noise-corrected roots are still generating, so the notebook cells are committed unexecuted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The figure sprawled: 15.2 x 6.8 inches at nearly 2.3:1, a full-width
italic footnote paragraph under the panels, a dead band above the
colorbar strip, and a legend wide enough to crowd the charged panel.
- Dropped the in-figure footnote. It was the largest single contributor
to the sprawl, and its content (the 2.47 kHz pricing, the M <= 1000
grid ceiling, Interp being excluded) is caption material, so it moved
to the markdown cell above the figures where a reader meets it before
the plot rather than squinting at it inside one.
- 15.2 x 6.8 -> 12.2 x 5.9 inches, with the panels closer together.
- Colorbar reserve 0.17 -> 0.12 for a single row, which is the value
plot_multi_method_window_sticker_component_panels and
plot_pareto_frontier_overlay already use, so this figure now matches
the rest of the paper's figures instead of carrying its own spacing.
- Smaller, tighter legend, and shorter curve labels ("Classical training
+ ibm_boston" rather than trailing "(Noise-Corrected)", which the
panel titles and colorbars already establish).
807 tests pass; flake8 clean. Both figures regenerated and checked.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both panels are identified in the paper caption, so the in-figure titles were redundant. plot_cost_model_comparison_panels gains show_titles (default True, so the function still titles panels for anyone calling it outside the paper figures) and both notebook cells pass False. The titles stay in the panel specs rather than being deleted: they label the progress output while the campaign roots load, which is how you tell which variant is being read. 807 tests pass; both figures regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous pass cut the figure from 15.2 to 12.2 inches wide, which overcorrected: the sprawl was the full-width footnote and the dead band above the colorbars, not the width. With those gone the panels read as squeezed, and four decades of log-scaled resource need the room. Back to 14.6 x 6.0 inches, keeping the reduced height. Colorbar reserve 0.13 so the strip still clears the axis labels at the shorter height. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… curve alone
Two corrections.
The prescription is an argmax under a cost model, so changing the cost
model has to change which (N, M, Q) is advised at each budget. Re-pricing
the points Fig. 12 already recommends answers "what does that
recommendation really cost", which is not the same question. Both panels
are therefore rebuilt from the exact points under their own cost model,
as they were before. Still no simulation re-run: the (N, M, Q) ->
approximation-ratio mapping carries no dependence on how resources are
priced.
The measured hardware curve is no longer charged. Its dur_mean is real
elapsed wall-clock -- classical training plus noise-corrected QPU time --
so it already contains whatever submission overhead those runs actually
incurred, and adding a modelled per-submission charge on top
double-counted it. It now carries the noise correction only, exactly as
in the single-panel figure, and is identical in both panels, which also
makes it a fixed reference the simulated curves move against.
apply_circuit_prep_to_prescription and circuit_submissions_for_n stay in
simulation_validation.py with their tests. They are what re-prices an
existing prescription without rebuilding anything, which is the source of
the paper-text figure ("the point reported at 0.12 s submits 16 circuits
and costs 222 s") even though it is no longer how the figure is built.
821 tests pass (14 new); flake8 clean; every notebook code cell compiles.
Cells committed unexecuted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…0,000 figure _draw_pareto_envelope_segments drew each owner's run over grid[start:stop], so consecutive segments shared no vertex and nothing spanned the takeover. Where the envelope steps up at the handover -- it is built from discrete (N, M, Q) points, so it can -- that showed as a visible break, most clearly at the Param. Transfer to Linear Ramp handover in the charged panel, which jumps 0.41 points at ~158 s. Each segment now reaches its successor's opening vertex, and the last run still stops at the end of the grid. The uncapped cost-model panel is dropped. Capping every family at Q <= 10,000 is the like-for-like comparison, so the notebook now produces that figure alone and its rationale moves into the surrounding markdown. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…l quimb Both families stop short of depth 7 in the resource-cost figure, so add the two Nautilus jobs that fill them in. Linear Ramp (LR_PP_opt) trains, so it runs 10-way sharded over the same N/M/Q grid as the p=9 campaign, followed by a finalize pass. Fixed Angles-dagger (FA_PP_no_opt) is zero-training and runs as a single pod over the extended Q grid its p=2..6 siblings used. The first submission failed on every shard with "No module named 'quimb'". qaoa_training_pipeline imports quimb from its evaluation package at import time and declares it in requirements.txt, but not in setup.py, so the "pip install -e" the runner used installed the package without it. Install the pipeline's declared requirements alongside it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…m dropped The previous fix read qaoa_training_pipeline/requirements.txt, which exists only in the older local checkout; upstream main moved its dependencies into pyproject.toml months ago and the file is gone, so every shard failed on "Could not open requirements file". quimb lives in the tns optional extra there, alongside cotengra and juliacall. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…text It is not going into the paper, so remove it rather than leave it as dead analysis. Out go the sqrt(gamma) machinery and Table V's N_CZ / sqrt(gamma) / t_nc / s_nc columns, the calib_nc_* frames, the noise-corrected window sticker, the noiseless-vs-noise-corrected Pareto overlay (both its cell and its markdown), and the dashed diamond curves in the two remaining figures. The measured hardware frontier moves onto the noiseless shot-rate basis, built from QPU_time_noiseless, so every curve in both figures is priced the same way and no part of the analysis still depends on the correction. plot_pareto_frontier_overlay is now uncalled anywhere; its import is dropped but the function is left in utils.py. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The third submission got past quimb and died on "cannot import name 'TRAINERS' from qaoa_training_pipeline.training". Upstream main removed that registry, which src/simulation_validation.py imports through ensure_pipeline_imports, so no campaign can run against the branch head at all. This also explains why the p=9 cluster campaign left only all_instance_specs.csv and a raw-points pickle. run_simulation_validation.sh now takes optional QPS_COMMIT and QAOA_PIPELINE_COMMIT pins and detaches to them after cloning. The p=7 jobs pin qaoa_training_pipeline to 5139dde and QAOA-Parameter-Setting to 1c43d95, the commits the local checkouts sit at, so the new depth-7 points come from the same code as the p=2..6 roots they will share a Pareto frontier with. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ists Dropping the noise-corrected calibration removed the "QPU_time_noise_corrected (s)" column from the notebook, but the helper still named it in a strict drop list, so the first hardware-frontier call after that change died with KeyError. Drop whichever QPU_time* columns the caller actually carries instead: the resource has already been folded into "total duration" by that point, and which calibration columns exist is the caller's business. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…sis.ipynb does The depth-graded colour ramps plus colorbars said depth twice, once per family, and did not match the recommendation plot the paper already uses. Switch both figures to that plot's scheme: one QPS colour per angle-setting method, depth as a "p=N" label on each takeover marker, and a single strategy legend where the colorbar strip used to be. Fixed Angles* and Fixed Angles-dagger share a colour under this scheme, so markers carry the recommendation plot's fill convention to separate them: hollow for a dagger method, filled with a dark edge for a starred one, filled and edgeless for method-parameter optimization. annotate_frontier_depths seeds its occupied-space list with the markers and rejects candidates outside the axes, so labels neither sit on the point they describe nor fall off the figure. The single-panel figure now routes through _draw_pareto_envelope_segments instead of its own inline copy, which also gives it the joined-at-the-takeover envelope fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…actly Fixed Angles-dagger and Fixed Angles-star do read as different shades in the recommendation plot: the dagger marker is hollow with a coloured rim, which next to the solid blue of the starred variant looks blueish white. That part was already right, but the rim convention was not applied faithfully elsewhere. _style_plot_kwargs gives a method-parameter-only method no rim at all, whereas these figures were drawing every such marker with a white one, so Param. Transfer and Linear Ramp carried an outline they should not have. Depth labels also now use the recommendation plot's #B00020 rather than grey. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The edit that collected hardware depth labels anchored on a line inside the
existing ax.scatter call, so the generator landed between "ax.scatter(" and
its arguments and the call was never closed. Move the extend above the
scatter, where it belongs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both figures set their limits to the exact data range, so the cheapest points sat on the left spine and the lowest ones on the bottom, leaving their depth labels nowhere to go. Reserve 0.16 decades left of the cheapest point (half that on the right, where the curves already run flat) and 6% of the y span above and below, exposed as x_pad_decades and y_margin. annotate_frontier_depths previously tried ten fixed offsets and, when none fit, dropped the label back onto the first one regardless of what was already there. It now sweeps five rings of eight directions and, only if every one of those collides, keeps whichever overlaps least and stays inside the axes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The annotator avoided markers and other labels but not the lines, so labels sat on top of the envelope and on error-bar whiskers wherever a takeover was close to a steep section. Every drawn line and whisker on the axes is now densified to 3 px spacing in display space and treated as an obstacle, with a penalty proportional to how many of those points a candidate bbox covers. Twelve directions over six rings give dense clusters enough slots that the least-bad fallback is rarely needed. Also adds heavy_hex_144_FA_no_opt_p7_expanded to COMPARISON_RESULT_TAGS; it had been generated and downloaded but never listed, so the figures could not see it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same top-left placement and axes-fraction anchor as plot_ibm_qaoa_performance_panels. The depth annotator now seeds its occupied-space list with any text already on the axes, so no p=N label can be pushed onto the panel letter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The recommendation plot in Analysis.ipynb insets its tightest group of frontier points so their depth labels have room; both frontier figures here now do the same. Unlike that plot, which hardcodes its cluster window, the cluster is found automatically: grow from the takeover point with the most neighbours within 42 px and take everything reachable through neighbour links, which is what a reader sees as one blob. The inset goes wherever it hides the least. Candidate positions are a 3 x 22 grid of axes-fraction slots; a slot is ruled out if it touches the cluster, the legend or the panel letter, and the rest are scored by drawn pixels covered, with a hidden marker costing 400 times a crossed line, since a curve is still readable from either side of an inset and a covered point is gone. Six named corners were not enough: panel (b)'s only clear region sits between its cluster and its legend, and the inset had to shrink to 0.28 of the axes width to fit there at all. Inside the inset the x axis is linear with plain numbers (the zoom window is a fraction of a decade, and log ticks came out as 10^-0.70), every main-axes curve is replotted without markers, the cluster's own markers are drawn in their family style, and the depth annotator runs on the inset alone. The main axes then label the remaining points with the inset bbox reserved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The connectors cut across the curves between the cluster and the inset. The rectangle on the main axes still says which region is zoomed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The rectangle on the main axes only cluttered the cluster it marked; the inset's own tick labels already say which region it shows. And the grid, at alpha 0.25 of the default light grey, was the same shade as the Param. Transfer curve. Major lines are now a distinctly darker grey, kept thin so they stay in the background, with fainter minor lines. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The bottom-left error bars and the topmost hardware whisker crossed the axis because the padding was computed from the envelope and marker values alone. Whisker extremes (CI bounds on the simulated takeovers, ar_sem on the hardware points) now count towards the y range in both figures, and the margin goes from 6% to 8% of that span. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Executed outputs only; no source changes. Both frontier figures now include Linear Ramp and Fixed Angles-dagger at p=7, strategy colours matching the recommendation plot, depth annotations, and the cluster inset. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…onda env name - Simulation notebook: drop four imports nothing in the body uses. - Analysis notebook: drop seven (matplotlib locators/formatters, Line2D, Plotting, ws_style, sem) likewise. - utils._family_marker_style: raw docstring; "$^\star$" was an invalid escape and warned on every import. - run_latency_recost: --variant-tag help still gave noise-correction as its example; describe the option by what it does instead. - run_job.sh: the conda env was hardcoded to "QAOA", which does not exist on this machine; default to "stochastic-benchmark" and honour CONDA_ENV. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An AST reachability pass over every top-level definition in examples/IBM_QAOA (roots: both notebooks' cell bodies with imports stripped, script entry points, shell and yaml, the core src/) found seven definitions no root reaches and twenty-two more that only tests/ mentions. This removes all of them, ~1,160 lines of src plus their 36 tests. utils.py (-806): plot_pareto_frontier_overlay, whose one caller was the deleted noiseless-vs-noise-corrected overlay cell; the legacy plotting set plot_training_bricks (superseded by plot_ibm_qaoa_training_bricks), plot_method_curves, plot_multi_method_window_sticker_components, _display_cross_strategy_envelope, title_from_instance_names, make_asof_per_file; and the window-sticker colour/label helpers window_sticker_method_color_map, _ws_display_method_label, _window_sticker_label_base, _window_sticker_label_depth, _shade_color, _percent_approx_ylabel, _percent_axis_values. simulation_validation.py (-350): the version-A re-pricing chain we chose not to use (apply_circuit_prep_to_prescription, circuit_submissions_for_n, COBYLA_EVALUATIONS_BY_N); sample_fixed_angles and sample_bound_circuit_counts; the budget-binning quartet build_binned_budget_dataset, build_budget_bin_edges, _centers_from_edges, _edges_from_centers; summarize_counts_metrics and counts_to_metric_rows; build_cost_operator_from_serialized; EVALUATOR_NAMES. Processing.cwd and approx_ratio_calc.counts_from_bitstring_samples likewise. Three imports that only those functions used go too, as do two README lines and the test-module docstrings that listed some of these as uncovered. The depth-gradient colorbar helpers stay: the multi-method window sticker in the notebook still draws them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
run_fa_no_opt_extended_q.sh and run_pt_fa_no_opt_extended.sh only replayed campaigns whose roots are on disk, with their Q grids recorded in each root's metadata. resource_cost_section.tex was a working draft for the paper and does not belong in the example directory. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every other manifest and the four manifest-generator scripts cloned both dependency repositories at their branch heads, which no longer works (qaoa_training_pipeline dropped the TRAINERS registry the campaign code imports), and each of their campaigns is either complete on disk or abandoned. The p=7 manifests pin both repositories to the commits the existing roots were produced with, so a new campaign starts from one of them. The README now walks through those three instead of the original FA_opt_p5 set, and explains the pins. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
bernalde
left a comment
There was a problem hiding this comment.
The cost comparison needs the corrections below before its numerical conclusions are ready to merge.
Blocking: Include the hardware submission cost in the charged comparison
In the notebook, cell 612c9536 replaces QPU time with num_shots / 2470, and _build_hw_frontier adds only classical total_train_cost. Cell c05tm0d2 then reuses that frontier unchanged in the charged panel, on the premise that it already includes submission overhead. It does not: executing these statements for 10,000 shots and zero classical cost plots 4.04858 s in both panels, even when the input measured QPU time is 20 s. The corresponding preparation-plus-shots model costs 17.91858 s.
Include one hardware preparation charge in the charged panel (the existing extra_cost path supports this), or supply measured end-to-end durations that actually include it. Update the explanation, regenerate the figure, and add a regression covering the hardware resource calculation alongside the simulated one.
Validation at 485eaa4:
- PR head:
.venv/bin/python -m pytest tests/ -q -rs(Python 3.13) — 784 passed, 4 skipped. The skips require Qiskit Aer, which is unavailable locally. - Merge result against
mainat7a8e013: its tree is identical to the PR head. The same suite in a temporary checkout with thestochastic-benchmark-ci-py310Python 3.10 environment — 784 passed, 4 skipped. Imports were verified to resolve to that checkout. - Both notebook import cells execute, all notebook code cells compile, targeted flake8 checks pass, and both changed shell scripts pass
bash -n. - Reproduced the cost discrepancies, rendered a synthetic comparison, inspected its plotted resources and the committed figure, and checked the two missing default campaign tags. The cost probe exercises
build_variantand the real frontier builder, with only the stochastic prescription stage stubbed. - Full campaign/notebook regeneration was not run: the required campaign pickles and hardware data are absent locally. The published campaign numbers remain unverified.
- Current-head GitHub checks pass: Python 3.10/3.11/3.12 tests, tutorial smoke, integration tests, and coverage summary.
This follows merged #84 and claims no issue closure. The existing resource-matching and bootstrap limitations remain tracked in #85 and #86. No other PR is currently open for coordination.
Blocking issues: 3 (2 inline, 1 in this body). Nonblocking issues: 0. Questions: 0.
I would not merge this until the blocking issues above are addressed.
| exact_df, | ||
| circuit_prep_time=circuit_prep_time, | ||
| time_per_shot=None, | ||
| use_recorded_shots=False, |
There was a problem hiding this comment.
Blocking: Make the charged variant implement the stated shot-cost equation
use_recorded_shots=False bills N*M training shots, while the equation in this script, the PR body, and the notebook bills n_evals*M. Through build_variant, a row with N=10, M=1000, Q=100, 15 evaluations, and 15,000 recorded training shots costs 226.00907 s, versus 228.03336 s under the stated equation at t_shot=1/2470 and t_prep=13.87. This switch also discards the interpolation baseline's recorded recursive shot count, so those variants do not differ solely by the preparation charge either.
Use the recorded training shots for the charged equation (the existing use_recorded_shots=True path reproduces it), reconcile the claim about what changes between the models, and regenerate the outputs. Add a regression through build_variant; the helper's default-path tests currently pass while the figure generator selects a different model.
| "heavy_hex_144_FA_no_opt_p2_expanded", | ||
| "heavy_hex_144_FA_no_opt_p3_expanded", | ||
| "heavy_hex_144_FA_no_opt_p4_expanded", | ||
| "heavy_hex_144_FA_no_opt_p6_expanded", |
There was a problem hiding this comment.
Blocking: Include both new depth-7 campaigns in the default regeneration
DEFAULT_TAGS omits heavy_hex_144_LR_opt_p7_expanded and heavy_hex_144_FA_no_opt_p7_expanded, although the notebook's COMPARISON_RESULT_TAGS includes both. Its documented generation command (python examples/IBM_QAOA/run_latency_recost.py --no-hardware-shot-times --q-cap 10000) supplies no --tags, so it never creates either campaign's baseline or charged roots. load_cost_model_panels reports missing roots and continues, producing an incomplete comparison that cannot reproduce the claimed Linear Ramp p=7 window.
Add both roots to the defaults, or share the campaign list between the script and notebook, and verify that the documented command generates both variants for every included campaign.
Executed outputs only; no source changes. Cells 1-13 in order, no errors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Everything on
examples/IBM_QAOAsince #84 merged: the resource-cost figure rework that came out of Daniel's and Davide's review of Fig. 12, the depth-7 campaigns it needed, and a dead-code pass over the whole example. 41 commits, 788 tests passing, flake8 clean.Figure 12: charge each submitted circuit for its preparation
Daniel measured that a single 144-node circuit takes ~13.87 s to return from
ibm_bostonregardless of shot count, and that COBYLA submits more circuits than its requestedmaxiter(N=10 → 15, N=100 → 114). The publishedT_proxycharged only shots, so it put fixed-angle optimisation at 0.1–0.2 s when the device cannot return a first sample in under ~10 s.T_proxy = t_pre + n_evals·(t_prep + M·t_shot) + (t_prep + Q·t_shot), withn_evalstaken from each run's recorded objective-evaluation count.run_latency_recost.pyre-prices the already-simulated points and redoes the cheap post-processing (~30 s per campaign).Depth-7 campaigns (Linear Ramp, Fixed Angles†)
Both families stopped at p=6 in the figure. The two Nautilus campaigns that fill them in are included as manifests, and both roots are on disk.
They also uncovered why the August p=9 campaign never finished:
qaoa_training_pipelineupstream has since removed theTRAINERSregistry thatsimulation_validation.pyimports, so any manifest tracking itsmainfails at import.run_simulation_validation.shnow takesQPS_COMMIT/QAOA_PIPELINE_COMMITpins and the p=7 manifests pin both repositories to the commits the existing roots were produced with. quimb is installed through the pipeline's[tns]extra.Figure styling
Both frontier figures now match
Analysis.ipynb's recommendation plot: one QPS colour per strategy, marker rims by optimisation level (hollow for †, black-rimmed for ★), depth as ap=Nlabel on each takeover,(a)/(b)panel letters, and the densest cluster zoomed into an inset. Depth labels are placed by a collision-avoiding annotator that treats markers, curves, whiskers, other labels and the inset as obstacles. Consecutive envelope segments now share their boundary vertex, which closes a one-grid-cell gap at every ownership handover that had been visible in the published figure.Notebook consolidation and cleanup
src/utils.pywith characterisation tests written first.src/) found 29 definitions nothing reaches. Removed with their 36 tests, ~1,160 lines. The depth-gradient colourbar helpers stay: the multi-method window sticker still draws them.run_job.shno longer hardcodes a conda env that does not exist.Paper text for the new figure, Table V and Equation (5) went to the author directly rather than into the repo.
🤖 Generated with Claude Code