Skip to content

feat: plot the pulse with Piccolo plot_pulse (not hand-rolled Makie) - #14

Merged
Rchari1 merged 1 commit into
mainfrom
rchari/template-plot-pulse
Jun 26, 2026
Merged

feat: plot the pulse with Piccolo plot_pulse (not hand-rolled Makie)#14
Rchari1 merged 1 commit into
mainfrom
rchari/template-plot-pulse

Conversation

@Rchari1

@Rchari1 Rchari1 commented Jun 17, 2026

Copy link
Copy Markdown
Member

Swaps the Run Inspector's per-iteration plot from hand-rolled CairoMakie to Piccolo's canonical plot_pulse. Stacked on #13 (inspector aesthetics); base rchari/inspector-aesthetics.

Why

The template's save_control_plot hand-rolled a Makie figure of prob.trajectory[:u] (raw control amplitudes vs timestep index) with one lines! per drive. Piccolo ships plot_pulse for exactly this — proper stacked per-drive panels, math-font labels, physical time axis, and optional drive-bound shading.

Change

solve_template.jl save_control_plot(k) becomes:

fig = plot_pulse(qcp; bounds = true, title = @sprintf("iter %d", k))
CairoMakie.save(@sprintf("iter_%04d.png", k), fig)
  • plot_pulse(qcp::QuantumControlProblem) reads the current optimizer iterate (kept in sync by callback_update_trajectory_factory), so frames stay live.
  • bounds=true shades the per-drive bounds.
  • Still cheap — no rollout, so the per-iter cadence (PLOT_EVERY) and the live solve speed are unchanged.
  • No new dependency: plot_pulse is in Piccolo, rendered via its Makie extension; CairoMakie (already a dep) stays the savefig backend. Net −5 lines.

Verification

Vetted end-to-end through amico-run on ~/.amico/julia (Piccolo 1.19):

  • X gate → F = 0.9997, 60 iters, status=completed, exit 0.
  • 7 of 7 frames distinct (md5) — confirms plot_pulse(qcp) tracks the live iterate, not a static initial pulse.
  • Extension build + 37 tests green (template is a bundled asset; no TS impact).

Pairs with #13: the inspector's branded "converged" panel now shows a canonical Piccolo pulse plot instead of raw line plots.

🤖 Generated with Claude Code

@jack-champagne jack-champagne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Gave it a look — clean one-file swap to Piccolo's plot_pulse(qcp; bounds=true), no new dep (Makie ext), CI green. Correct as-is, nothing blocking.

Taste / non-blocking

  • Heads-up: the Phase 0′ callback work (#19/#21) replaces this per-iter plotting with AbstractIntermediateCallback/LivePulsePlotCallback, so this polish lands on a scaffold we're about to swap. Fine to take now — just flagging so we don't double-invest.

…ed Makie

Replace the hand-rolled CairoMakie lines! plot of prob.trajectory[:u] with
Piccolo's canonical plot_pulse(qcp; bounds=true) — proper stacked per-drive
pulse panels with the drive bounds shaded, in math-font labels. Still cheap
(no rollout), still per-PLOT_EVERY + final frame.

Vetted through amico-run on ~/.amico/julia (Piccolo 1.19): X gate F=0.9997,
60 iters, 7 distinct frames (confirms plot_pulse(qcp) tracks the live iterate),
status=completed. CairoMakie stays the savefig backend; no new dependency
(plot_pulse ships in Piccolo, rendered via its Makie extension).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Rchari1
Rchari1 force-pushed the rchari/inspector-aesthetics branch from fd9bded to b64dcb8 Compare June 21, 2026 01:46
@Rchari1
Rchari1 force-pushed the rchari/template-plot-pulse branch from b39dcd4 to 9b287ad Compare June 21, 2026 01:46
@jack-champagne

Copy link
Copy Markdown
Member

LGTM. Clean swap to Piccolo's plot_pulse, net-negative LOC, and — the thing that could have broken the inspector — the iter_<N>.png filename, write location, per-iter callback, and PLOT_EVERY cadence are all preserved, so the watcher's frame discovery is unaffected. Frames stay live (plot_pulse reads qcp.prob.trajectory, the object the callback mutates).

Two minors, neither blocking: the body calls plot_pulse a "Makie extension" — it's a normal Visualizations submodule (works because using CairoMakie is already loaded), so the "no new dep" rationale is slightly off. And bounds=true adds a path that, under the bare try/catch, would swallow into zero frames if a future system's drive_bounds shape mismatches — consider a narrower catch or a bounds=false fallback.

@jack-champagne jack-champagne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving — clean swap, and the iter_*.png frame contract is preserved. Two non-blocking minors above.

@Rchari1
Rchari1 changed the base branch from rchari/inspector-aesthetics to main June 26, 2026 20:10
@Rchari1
Rchari1 merged commit 5d2cbe8 into main Jun 26, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants