0.2c — Route per-iter PNGs through LivePulsePlotCallback (AbstractIntermediateCallback) - #43
Conversation
…ctIntermediateCallback) Retire the hand-rolled `plot_pulse`/`CairoMakie.save` per-iter PNG path (the PR #14 interim) in the bundled solve template. The live plot now flows through Piccolo's `LivePulsePlotCallback` — an `AbstractIntermediateCallback` — installed on the Ipopt solve via `IpoptOptions(intermediate_callback=…)` (the symbol DTO 0.2a wires into the Ipopt backend). This satisfies the Phase-0prime DoD: "β's live plot now flows through AbstractIntermediateCallback." - templates/solve_template.jl: - `LivePulsePlotCallback(qtraj, prob.trajectory; every=PLOT_EVERY, save_dir=".")` installed via `options=IpoptOptions(intermediate_callback=live_plot)`; emits `iter_<N>.png` through the callback abstraction (inspector unchanged — it reads run-dir PNGs; its regex already accepts unbounded digits). - AMICODE_ITER text telemetry stays INTACT on the raw Ipopt callback (it needs the rich IPM state obj_value/inf_pr/inf_du the agnostic `(primal,iter)` contract can't carry). DTO composes the raw callback with the intermediate_callback so both fire per iter. - Drop `save_control_plot` + the end-of-solve fallback plot: LivePulsePlotCallback fires at iter 0 and every PLOT_EVERY, so a run always has ≥1 frame, and a direct plot would defeat routing every frame through the callback. - `callback_update_trajectory_factory` dropped — LivePulsePlotCallback already reconstructs the trajectory from the primal each iter. - AGENTS.md: document `LivePulsePlotCallback`/`AbstractIntermediateCallback` as the blessed per-iter plot idiom; tell the agent never to hand-roll plotting. Live inspector is ipopt-only for v1 (Q74). Repo grep confirms no plot_pulse/save_control_plot/CairoMakie.save remains in the template; template parses clean. NOTE: cannot go green end-to-end until DTO 0.2a is released (v0.9.7, the intermediate_callback field) and the bundled julia Manifest (packages/extension/julia) is bumped to resolve it — a release-coordination follow-up. Fast-lane CI is unaffected (it doesn't execute the template; the template-run validation is the slow tier). Refs #21. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jack-champagne
left a comment
There was a problem hiding this comment.
Read this against the pinned Julia env + the depot source, and the end-state is the right one — routing per-iter PNGs through LivePulsePlotCallback is exactly the 0.2c target. I confirmed the callback renders the convention-correct way (extract_pulse(qtraj, traj) → plot_pulse(pulse; …) in PiccoloMakieExt, not raw plot(traj,[:u])), and it zero-pads iter_00006.png, so the inspector's iter_(\d+) watcher still orders frames fine. The AGENTS.md idiom section is clean.
The problem is sequencing: this is the 0.2c consumer landing ahead of its 0.2a producer. The bundled packages/extension/julia Manifest pins DirectTrajOpt 0.9.6, and I checked the depot — intermediate_callback is a MadNLPOptions-only field there (the Ipopt path never invokes an AbstractIntermediateCallback; wiring it into Ipopt is literally #19). So IpoptOptions(intermediate_callback = live_plot) throws at construction against the env we actually ship — the bundled template is dead-on-arrival — and nothing in CI catches it (the fast job excludes **/slow/**, schema-roundtrip is non-Piccolo, boot-smoke is opencode-only, and test/slow/template.test.ts self-skips without AMICO_TEST_JULIA_PROJECT). It also runs against D10, which says hand-roll the β PNG from the existing Ipopt callback and treat the abstraction as fast-follow because it's absent from DTO.
Three changes before merge (inline):
- [blocking] gate on the DTO v0.9.7 release (#19) and bump
packages/extension/julia/Manifest.tomlto it — ideally in this PR so the template + the pin move in lockstep (same rule as the #28/#30 rename). - [blocking] keep the hand-rolled
plot_pulse/savefallback documented in AGENTS.md until that pin lands — don't ban the one idiom that runs on 0.9.6, which is also the one D10 prescribes for β. - [important] add a slow-tier CI gate that runs the bundled template ≥1 iteration against
packages/extension/julia(same hole as #45), so "template doesn't run on the pin" can't merge green again.
Verify against v0.9.7 once it's pinned (not blocking the above, but don't rely on them until checked): that Ipopt actually invokes the callback at iter 0 (so dropping the end-of-solve fallback is safe for sub-PLOT_EVERY solves), and that the raw callback (AMICODE_ITER) and intermediate_callback (PNG) both fire per iter on Ipopt.
Taste / non-blocking: the live frames lose the bounds=true shading the old plot_pulse(qcp; bounds=true) gave — LivePulsePlotCallback hardcodes plot_pulse(pulse; title) with no bounds knob, so a researcher watching a solve can't see the drive railing against its amplitude limit anymore. Minor, but it's a real downgrade in the live view.
…otCallback once DTO≥0.9.7 pinned) Review (Jack #43): don't ban the hand-rolled Ipopt-callback PNG path — it's the only one that runs on the pinned DTO 0.9.6 and the one D10 prescribes for β. Soften the idiom guidance to a conditional: - prefer LivePulsePlotCallback once the bundled Julia project pins DirectTrajOpt ≥ 0.9.7 (the Ipopt intermediate_callback field); - otherwise hand-roll plot_pulse/CairoMakie.save from the raw Ipopt callback (IpoptOptions(intermediate_callback=…) throws at construction on 0.9.6). Note the bundled template's LivePulsePlotCallback path lands in lockstep with the DTO ≥0.9.7 Manifest bump, so template + pin never diverge on main. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed the review — converted to draft, since this is hard-blocked on the DTO v0.9.7 release. What landed now (c860d28): AGENTS.md softened to the dual-state idiom you asked for (prefer Lockstep plan (the remaining blocking items move together):
The flipped template is already validated end-to-end against a dev-coupled DTO#114 on gpu-dev-v2 (11 |
|
Correction (retract the prior note): DTO 0.9.7 is the correct pin — the callback fix was folded into 0.9.7 after all. Release is imminent (tonight). When 0.9.7 registers: bump |
|
DTO 0.9.7 is out — bumped Re-review still owes (from the prior pass, needs a real solve): (1) confirm Ipopt actually invokes the callback at iter 0 and both callbacks (LivePulsePlotCallback + cb_log) fire; (2) the template dropped the end-of-solve guarantee frame — a solve shorter than PLOT_EVERY could leave the inspector blank; (3) bounds=true shading loss is non-blocking. |
…view #43) Jack's re-review item (2): the migration dropped the end-of-solve frame, so a solve that converges in < PLOT_EVERY iters would leave only the iter-0 random-init frame. Restore the guarantee — but keep it flowing through LivePulsePlotCallback (no bespoke plot_pulse): re-invoke the callback once at every=1 with the FINAL primal (reconstructed from prob.trajectory, which DTO syncs to the final iterate after solve!). Short solves now end on the converged pulse; typical solves overwrite the last multiple-of-PLOT_EVERY frame with the same content. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Re-review items verified on the released DTO 0.9.7 (thanks for the Manifest bump). Ran the bundled template against Item 1 — Ipopt invokes the callback at iter 0, and both callbacks fire. Normal solve (
Item 2 — end-of-solve guarantee frame (dc602ea). Short solve (
Item 3 — CI is green on the fast lanes. Ready for another look. |
|
Verified end-to-end on DTO 0.9.7 — ran the vetted template (X gate, 3-level transmon) via
All prior review comments now addressed: AGENTS.md fallback ( Non-blocking leftover: a cosmetic |
jack-champagne
left a comment
There was a problem hiding this comment.
All four review comments addressed and verified end-to-end on DTO 0.9.7 (both callbacks fire incl. iter 0, guarantee frame writes the converged pulse, fidelity 0.99999, watcher parses the frames). Threads resolved, CI green.
Implements #21 (Phase 0′, task 0.2c). Depends on 0.2a (DirectTrajOpt.jl#114 + Piccolo.jl install path).
What this does
Retires the hand-rolled
plot_pulse/CairoMakie.saveper-iter PNG path (the PR #14 interim) in the bundled solve template. The live plot now flows through Piccolo'sLivePulsePlotCallback— anAbstractIntermediateCallback— installed on the Ipopt solve viaIpoptOptions(intermediate_callback=…). Satisfies the Phase-0prime DoD: "β's live plot now flows throughAbstractIntermediateCallback."LivePulsePlotCallback(qtraj, prob.trajectory; every=PLOT_EVERY, save_dir=".")installed viaoptions=IpoptOptions(intermediate_callback=live_plot); emitsiter_<N>.pngthrough the callback abstraction (inspector unchanged — its regex already accepts unbounded digits).obj_value/inf_pr/inf_du) the agnostic(primal,iter)contract can't carry. DTO composes the raw callback withintermediate_callback, so both fire per iter.save_control_plot+ the end-of-solve fallback plot (LivePulsePlotCallback fires at iter 0 + every PLOT_EVERY → always ≥1 frame) and the now-redundantcallback_update_trajectory_factory(the callback reconstructs the trajectory from the primal itself).LivePulsePlotCallback/AbstractIntermediateCallbackas the blessed per-iter plot idiom; instructs the agent never to hand-roll plotting.Live inspector is ipopt-only for v1 (Q74). Repo grep confirms no
plot_pulse/save_control_plot/CairoMakie.saveremains in the template.Validation (gpu-dev-v2, end-to-end)
Ran the migrated template against a dev-coupled DTO#114 + Piccolo install-path env: exit 0 · 11
iter_*.pngframes · 61AMICODE_ITERlines ·result.tomlwritten ·DONEemitted. Per-iter PNGs flow through the callback and AMICODE_ITER survives.Cannot go green end-to-end in CI until DTO 0.2a is released (v0.9.7, the
intermediate_callbackfield) and the bundledpackages/extension/juliaManifest (currently DTO v0.9.6) is bumped to resolve it. Chain: merge DTO#114 → register v0.9.7 → (Piccolo install-path PR) → bump amicode's julia Manifest. Fast-lane CI here is unaffected (it doesn't execute the template; that's the slow tier).🤖 Generated with Claude Code