From 95483657fd585218772eaf1587aafbf2c51e99b8 Mon Sep 17 00:00:00 2001 From: PhotonicVelocity Date: Tue, 16 Jun 2026 17:58:42 -0700 Subject: [PATCH 1/6] fix(run_sim): snap _anim_frame to k*dt grid to prevent drift _anim_frame reschedules via `t + _dt`, so the scheduled time accumulates FP error and drifts below the k*dt grid by more than the interval-overlap dedup tol (1e-15) around tick 93 at fps=30. The dedup in _interval_hybrid then stops recognizing result.t[0] as a duplicate and every subsequent interval double-logs its start sample. Snap the next event to k*_dt by recomputing the tick index from current t: `(round(t / _dt) + 1) * _dt`. No accumulation, no drift. --- src/bdsim/run_sim.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/bdsim/run_sim.py b/src/bdsim/run_sim.py index 274e1a4..436544d 100644 --- a/src/bdsim/run_sim.py +++ b/src/bdsim/run_sim.py @@ -1461,8 +1461,10 @@ def _anim_frame(t: float, ss: Any, _dt: float = interactive_dt) -> None: time.sleep(min(notebook_min_refresh_dt, 0.05)) if getattr(ss, "stop", None) is not None: return - if t + _dt < tf - event_tol: - ss.declare_event(_anim_frame, t + _dt) + # Schedule the next anim_frame on the canonical k*_dt grid. + next_t = (round(t / _dt) + 1) * _dt + if next_t < tf - event_tol: + ss.declare_event(_anim_frame, next_t) # Schedule frame callbacks for all animated runs; notebook mode # relies on these callbacks to refresh inline figure output. From 1caebd1e13c6bac43620748a3b2591f46836a72f Mon Sep 17 00:00:00 2001 From: PhotonicVelocity Date: Tue, 16 Jun 2026 18:12:45 -0700 Subject: [PATCH 2/6] fix(run_sim): capture t=0 IC sample in hybrid-diagram runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The interval-overlap dedup at the top of _interval_hybrid unconditionally skips result.t[0] when it matches t0, which on the first call drops the t=0 IC sample for every continuous-state diagram — tlist[0] ends up at 1/fps, watch logs miss the IC, and the live window / movie writer start from the first integration step. Mirror the t=0 pre-pass the discrete-only path already does: evaluate the diagram at (t=0, y=x0) and call _record_sample_and_service_hooks so the IC lands in tlist, xlist, watch logs, and sinks. Also call display_manager.refresh() once so flush_events() and any movie writer's grab_frame() pick up the IC frame (the refresh hook only normally fires from _anim_frame, scheduled at interactive_dt, not 0). --- src/bdsim/run_sim.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/bdsim/run_sim.py b/src/bdsim/run_sim.py index 436544d..59a6bad 100644 --- a/src/bdsim/run_sim.py +++ b/src/bdsim/run_sim.py @@ -1470,6 +1470,26 @@ def _anim_frame(t: float, ss: Any, _dt: float = interactive_dt) -> None: # relies on these callbacks to refresh inline figure output. simstate.declare_event(_anim_frame, interactive_dt) + # Mirror the discrete-only t=0 pre-pass so hybrid diagrams capture + # the IC sample (the per-interval dedup skips result.t[0]=0). + if bd.nstates > 0: + simstate.t = 0.0 + simstate.count += 1 + eval_start = time.time() + bd.evaluate(bd.state_map(x0, simstate), 0.0, sinks=False) + simstate.bdtime += time.time() - eval_start + self._record_sample_and_service_hooks( + bd, simstate, 0.0, x0, stop_short_circuit=False, + ) + # refresh() (present + grab_frame) only fires from _anim_frame + # at t=interactive_dt — call it once so the IC reaches the live + # window and the movie writer. + if ( + simstate.options.animation + and simstate.display_manager is not None + ): + simstate.display_manager.refresh() + while t0 < tf - event_tol: # Next scheduled boundary (clock tick, explicit event, or terminal marker). tnext, sources = simstate.eventq.pop(dt=1e-6) From 24f8e1c044b75a164dd66c9216b095b97aff6b04 Mon Sep 17 00:00:00 2001 From: PhotonicVelocity Date: Tue, 16 Jun 2026 18:32:17 -0700 Subject: [PATCH 3/6] fix(run_sim): treat solve_ivp natural completion as reaching t1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two coupled bugs fire on every interval whose t_eval grid doesn't end at t1 (every non-dt-aligned boundary): 1. `_interval_hybrid` returns `treached = result.t[-1]` — the last OUTPUT sample, not the integrator's reach. On natural completion the solver always reached t1, but result.t[-1] is the last t_eval point before t1. The main loop sees treached < t1, bumps t0 by event_tol, and re-enters integration on a tiny residual. The residual call builds an empty t_eval grid, falls through to a t_eval-less solve_ivp call, and dumps every natural integrator step into result.t — ~33 samples per boundary at max_step=1e-3, 100+ on non-trivial graphs with adaptive RK45. Use t1 for treached when result.status == 0; fall back to result.t[-1] only for event-terminated runs (status == 1). 2. With treached == t1 the spurious residual call is gone, but the sample loop only logs t_eval points — non-dt-aligned boundaries no longer land in tlist at all. Artists don't update at the boundary moment and watch logs miss the boundary times. Extend t_eval to include t1 when it isn't already on the dt grid. --- src/bdsim/run_sim.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/bdsim/run_sim.py b/src/bdsim/run_sim.py index 59a6bad..baaf2aa 100644 --- a/src/bdsim/run_sim.py +++ b/src/bdsim/run_sim.py @@ -1849,6 +1849,14 @@ def ydot(t: float, y: np.ndarray) -> np.ndarray: if simstate.dt is not None: t_eval = self._build_t_eval_grid(float(t0), float(t1), float(simstate.dt)) if t_eval is not None: + # Always include t1 so boundary events at non-dt-multiple + # times (clock ticks, _anim_frame callables) get a sink-tick + # — otherwise sinks freeze at the last dt-grid sample. + if not np.isclose(float(t_eval[-1]), float(t1), rtol=0.0, atol=1e-12): + t_eval = np.concatenate([ + t_eval, + np.array([float(t1)]) + ]) ivp_args.setdefault("t_eval", t_eval) # Keep user-provided method if present; otherwise use option/default, @@ -1966,8 +1974,14 @@ def ydot(t: float, y: np.ndarray) -> np.ndarray: crossing_state_map, ) - # return final continuous state and actual end time reached - t_final = float(result.t[-1]) if len(result.t) > 0 else float(t0) + # return final continuous state and actual end time reached. + # `result.t[-1]` is the last OUTPUT sample, not the integrator's + # reach; on natural completion it can undershoot t1 and trigger a + # spurious residual call. Use t1 directly when status == 0. + if result.status == 0: + t_final = float(t1) + else: + t_final = float(result.t[-1]) if len(result.t) > 0 else float(t0) if len(result.t) > 0: if crossing_handled: x_final = bd.continuous_state_vector(crossing_state_map) From 3b0ada1bd05ef4a2e47804061330da992d2a0b21 Mon Sep 17 00:00:00 2001 From: PhotonicVelocity Date: Tue, 16 Jun 2026 19:10:32 -0700 Subject: [PATCH 4/6] fix(graphics): grab MP4 frames at fps cadence, not per sink tick MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GraphicsBlock.step called writer.grab_frame() on every sink tick, so the MP4 ended up with one frame per integration time point — at fps=30 with a non-trivial integrator that's ~200 frames per 5 s of sim, a 6.9 s video at 30 fps writer rate. Move the grab to DisplayManager.refresh(), which already runs at interactive_dt cadence from _anim_frame (the same hook that calls flush_events() for live windows). _start_movie attaches the writer to fig._bdsim_movie_writer; a new _grab_movie_frame() helper in display.py picks it up per figure during refresh. Also widen the _anim_frame reschedule guard from `next_t < tf - tol` to `next_t <= tf + tol` so a frame landing exactly on tf still fires. --- src/bdsim/block.py | 8 ++------ src/bdsim/display.py | 22 ++++++++++++++++++++++ src/bdsim/run_sim.py | 3 ++- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/bdsim/block.py b/src/bdsim/block.py index 200a038..c759ff6 100644 --- a/src/bdsim/block.py +++ b/src/bdsim/block.py @@ -2157,6 +2157,8 @@ def _start_movie(self) -> None: fps=fps_int, extra_args=["-vcodec", "libx264"] ) self._writer.setup(fig=self._fig, outfile=self._movie) # type: ignore[union-attr] + # Attach writer to figure so DisplayManager.refresh() can find it for fps-paced grabs. + self._fig._bdsim_movie_writer = self._writer # type: ignore[union-attr] self._movie_started = True print("movie block", self, " --> ", self._movie) except FileNotFoundError: @@ -2226,12 +2228,6 @@ def step(self, t: float, inports: list[Any]) -> None: else: self._fig.canvas.draw() # type: ignore[union-attr] - if self._movie is not None: - try: - self._writer.grab_frame() # type: ignore[union-attr] - except AttributeError: - self.fatal("cannot save movie, please install ffmpeg") # type: ignore[union-attr] - def done(self, block=False, **kwargs) -> None: if self._fig is not None: simstate = getattr(self, "_simstate", None) diff --git a/src/bdsim/display.py b/src/bdsim/display.py index 6c91a39..5f6d142 100644 --- a/src/bdsim/display.py +++ b/src/bdsim/display.py @@ -55,6 +55,24 @@ import matplotlib.pyplot as plt +def _grab_movie_frame(fig: Any) -> None: + """Grab one frame for any MP4 writer attached to ``fig``. + + The writer is set on ``fig._bdsim_movie_writer`` by + ``GraphicsBlock._start_movie`` when a movie path is configured. Co-locating + the grab with the display refresh keeps the movie's frame cadence on the + fps grid (the same hook that drives ``flush_events()`` for live windows), + regardless of how many sink ticks the integrator emits in between. + """ + writer = getattr(fig, "_bdsim_movie_writer", None) + if writer is None: + return + try: + writer.grab_frame() + except AttributeError: + pass + + class DisplayManager: """Abstract base class and factory for bdsim display managers. @@ -228,6 +246,7 @@ def refresh(self) -> None: handle.update(fig) except Exception: pass + _grab_movie_frame(fig) def refresh_figure(self, fig: Any) -> None: """Refresh one figure, including figures not listed by pyplot.""" @@ -242,6 +261,7 @@ def refresh_figure(self, fig: Any) -> None: handle.update(fig) except Exception: pass + _grab_movie_frame(fig) def finalize(self, hold: bool = False) -> None: """Render a final frame and close all registered figures. @@ -279,10 +299,12 @@ def refresh(self) -> None: for fig in self._iter_figures(): fig.canvas.draw_idle() fig.canvas.flush_events() + _grab_movie_frame(fig) def refresh_figure(self, fig: Any) -> None: fig.canvas.draw_idle() fig.canvas.flush_events() + _grab_movie_frame(fig) def finalize(self, hold: bool = False) -> None: if hold: diff --git a/src/bdsim/run_sim.py b/src/bdsim/run_sim.py index baaf2aa..ab6cee9 100644 --- a/src/bdsim/run_sim.py +++ b/src/bdsim/run_sim.py @@ -1462,8 +1462,9 @@ def _anim_frame(t: float, ss: Any, _dt: float = interactive_dt) -> None: if getattr(ss, "stop", None) is not None: return # Schedule the next anim_frame on the canonical k*_dt grid. + # `<= tf + tol` so a frame landing exactly on tf still fires. next_t = (round(t / _dt) + 1) * _dt - if next_t < tf - event_tol: + if next_t <= tf + event_tol: ss.declare_event(_anim_frame, next_t) # Schedule frame callbacks for all animated runs; notebook mode From 7c702da3835cd01f6e905410cafe17182b6d581f Mon Sep 17 00:00:00 2001 From: PhotonicVelocity Date: Tue, 16 Jun 2026 19:40:11 -0700 Subject: [PATCH 5/6] test(hybrid-sim): add regression coverage for per-interval grid + movie cadence - HybridSimSampleGridTest (test_run_sim.py) pins the four per-interval sample-grid behaviors: t=0 IC, no FP drift duplicates, anim_frame boundaries in tlist, no residual sample explosion. - New unit tests for `_grab_movie_frame` (test_display_manager.py): no-op when no writer, grab when attached, swallow AttributeError. - Drop `test_step_movie_no_writer_attribute_error` (pins removed per-tick `grab_frame()` path) and the grab_frame assertion in `test_step_timestamp_overlay_updates`. --- tests/test_display_manager.py | 27 ++++++++++++++++++++ tests/test_graphics.py | 12 +-------- tests/test_run_sim.py | 47 +++++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 11 deletions(-) diff --git a/tests/test_display_manager.py b/tests/test_display_manager.py index 6587c97..8558ff9 100644 --- a/tests/test_display_manager.py +++ b/tests/test_display_manager.py @@ -7,13 +7,40 @@ matplotlib.use("Agg") +from unittest.mock import MagicMock + from bdsim.display import ( DisplayManager, MatplotlibDisplayManager, NotebookDisplayManager, + _grab_movie_frame, ) +def test_grab_movie_frame_no_writer_is_noop() -> None: + """A figure with no `_bdsim_movie_writer` attribute is a silent no-op.""" + fig = SimpleNamespace() + _grab_movie_frame(fig) # must not raise + + +def test_grab_movie_frame_with_writer_grabs_once() -> None: + """When a writer is attached, `_grab_movie_frame` calls `grab_frame()` once.""" + writer = MagicMock() + fig = SimpleNamespace(_bdsim_movie_writer=writer) + _grab_movie_frame(fig) + writer.grab_frame.assert_called_once() + + +def test_grab_movie_frame_swallows_attribute_error() -> None: + """If the writer is broken (no grab_frame), the helper stays quiet.""" + # Object without a grab_frame attribute raises AttributeError when called. + class BrokenWriter: + pass + + fig = SimpleNamespace(_bdsim_movie_writer=BrokenWriter()) + _grab_movie_frame(fig) # must not raise + + def test_factory_returns_notebook_manager() -> None: manager = DisplayManager.create(notebook_backend=True) assert isinstance(manager, NotebookDisplayManager) diff --git a/tests/test_graphics.py b/tests/test_graphics.py index 49d60e9..767f1b7 100644 --- a/tests/test_graphics.py +++ b/tests/test_graphics.py @@ -181,17 +181,8 @@ def test_step_tiled_shared_figure_draws_once_per_time(self): finally: plt.close("all") - def test_step_movie_no_writer_attribute_error(self): - """step() with movie set but no writer → AttributeError in grab_frame (lines 75-79).""" - gb = MinGB(nin=1, movie="out.mp4") - ss = _make_simstate(animation=False) - gb._simstate = ss - # self._writer never set → AttributeError from grab_frame except block → fatal() → AttributeError - with self.assertRaises(AttributeError): - gb.step(0.0, [1.0]) - def test_step_timestamp_overlay_updates(self): - """step() updates the timestamp text and then grabs a movie frame.""" + """step() updates the timestamp text artist.""" gb = MinGB(nin=1, movie="out.mp4", timestamp=True) ss = _make_simstate(animation=False) gb._simstate = ss @@ -203,7 +194,6 @@ def test_step_timestamp_overlay_updates(self): gb.step(1.2345, [1.0]) self.assertIsNotNone(gb._timestamp_artist) self.assertEqual(gb._timestamp_artist.get_text(), "t=1.234") - gb.writer.grab_frame.assert_called_once() finally: plt.close("all") diff --git a/tests/test_run_sim.py b/tests/test_run_sim.py index 37105e4..18bb6ea 100644 --- a/tests/test_run_sim.py +++ b/tests/test_run_sim.py @@ -1364,5 +1364,52 @@ def test_cli_set_empty_by_default(self): self.assertEqual(opts.setglob, []) +# --------------------------------------------------------------------------- +class HybridSimSampleGridTest(unittest.TestCase): + """Regression tests for the per-interval sample grid in hybrid runs. + + Each test sets up a trivial `CONSTANT → INTEGRATOR` diagram with + animation enabled. The integrator gives the diagram continuous state + (so `_interval_hybrid` is exercised); the diagram itself is trivial + so the sample grid is determined entirely by `dt`, the anim_frame + cadence, and the integration loop's sample-logging behavior. + """ + + @staticmethod + def _run(T: float, dt: float, fps: float, **run_kwargs): + sim = bdsim.BDSim(animation=True, animation_rate=fps, quiet=True) + bd = sim.blockdiagram() + bd.connect(bd.CONSTANT(1.0), bd.INTEGRATOR(0.0)) + bd.compile() + return sim.run(bd, T=T, dt=dt, **run_kwargs) + + def test_out_t_starts_at_zero(self): + """The IC sample at t=0 lands in out.t.""" + out = self._run(T=1.0, dt=1 / 30, fps=30) + self.assertEqual(float(out.t[0]), 0.0) + + def test_no_drift_doubled_samples(self): + """Past the FP drift threshold, out.t still has the expected inclusive + dt-grid count and contains no duplicate timestamps.""" + out = self._run(T=3.5, dt=1 / 30, fps=30) + self.assertEqual(len(out.t), 106) # 3.5 * 30 + 1 + self.assertGreater(float(np.diff(out.t).min()), 0.0) + + def test_anim_frame_boundary_in_tlist(self): + """Non-`dt`-aligned anim_frame boundaries land in out.t.""" + out = self._run(T=1.0, dt=0.1, fps=3) + for boundary in (1 / 3, 2 / 3): + self.assertTrue( + np.any(np.isclose(out.t, boundary, atol=1e-9)), + f"expected t={boundary:.4f} in out.t, got {list(out.t)}", + ) + + def test_no_residual_sample_explosion(self): + """Small `max_step` on a non-`dt`-aligned boundary does not dump every + integrator step into out.t.""" + out = self._run(T=1.0, dt=0.1, fps=3, max_step=1e-3) + self.assertEqual(len(out.t), 13) # 11 dt-grid + 2 anim_frame boundaries + + if __name__ == "__main__": unittest.main() From 9b03bcb760c287c22e17924a67dd586e8a607835 Mon Sep 17 00:00:00 2001 From: PhotonicVelocity Date: Tue, 16 Jun 2026 20:28:49 -0700 Subject: [PATCH 6/6] fix(run_sim): always include endpoints in _build_t_eval_grid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Returning None from _build_t_eval_grid (when no k*dt multiple lands in [t0, t1]) left ivp_args.t_eval unset, so solve_ivp fell back to natural per-step output — the same residual-sample explosion the previous fix closed for natural completion, but reachable through event-terminated re-entries, cross-domain timing (clock period not a multiple of dt), tf near a scheduled boundary, and near-simultaneous events. Change the contract: always return a non-empty grid containing the endpoints t0 and t1 plus any k*dt multiples in between. The "extend t_eval to include t1" block at the call site collapses into the builder. Caller already guarantees dt > 0 (validated at run start) and t0 < t1 (while-loop guard + event_tol skip). --- src/bdsim/run_sim.py | 38 +++++++++++++++----------------------- tests/test_run_sim.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/src/bdsim/run_sim.py b/src/bdsim/run_sim.py index ab6cee9..e7c0ce8 100644 --- a/src/bdsim/run_sim.py +++ b/src/bdsim/run_sim.py @@ -880,22 +880,23 @@ def _solve_ivp_method(self, simstate: BDSimState) -> str: return str(simstate.solver) @staticmethod - def _build_t_eval_grid(t0: float, t1: float, dt: float) -> np.ndarray | None: - """Build an absolute-time t_eval grid over the closed interval [t0, t1].""" + def _build_t_eval_grid(t0: float, t1: float, dt: float) -> np.ndarray: + """Build an absolute-time t_eval grid over the closed interval [t0, t1]. + + The grid always contains the endpoints `t0` and `t1` plus any `k*dt` + multiples that land strictly between them. `dt > 0` and `t0 < t1` are + guaranteed by the caller. + """ tol = 1e-12 - if dt <= 0: - return None k0 = int(np.ceil((t0 - tol) / dt)) k1 = int(np.floor((t1 + tol) / dt)) - if k1 < k0: - return None - grid = dt * np.arange(k0, k1 + 1, dtype=float) - grid = grid[(grid >= (t0 - tol)) & (grid <= (t1 + tol))] - if grid.size == 0: - return None - grid = np.clip(grid, t0, t1) - grid = np.unique(grid) - return grid if grid.size > 0 else None + if k1 >= k0: + interior = dt * np.arange(k0, k1 + 1, dtype=float) + interior = interior[(interior >= (t0 - tol)) & (interior <= (t1 + tol))] + interior = np.clip(interior, t0, t1) + else: + interior = np.array([], dtype=float) + return np.unique(np.concatenate([[t0], interior, [t1]])) def _dispatch_crossing_event( self, @@ -1849,16 +1850,7 @@ def ydot(t: float, y: np.ndarray) -> np.ndarray: if simstate.dt is not None: t_eval = self._build_t_eval_grid(float(t0), float(t1), float(simstate.dt)) - if t_eval is not None: - # Always include t1 so boundary events at non-dt-multiple - # times (clock ticks, _anim_frame callables) get a sink-tick - # — otherwise sinks freeze at the last dt-grid sample. - if not np.isclose(float(t_eval[-1]), float(t1), rtol=0.0, atol=1e-12): - t_eval = np.concatenate([ - t_eval, - np.array([float(t1)]) - ]) - ivp_args.setdefault("t_eval", t_eval) + ivp_args.setdefault("t_eval", t_eval) # Keep user-provided method if present; otherwise use option/default, # then finally derive from run(..., solver=...). diff --git a/tests/test_run_sim.py b/tests/test_run_sim.py index 18bb6ea..66ef4f2 100644 --- a/tests/test_run_sim.py +++ b/tests/test_run_sim.py @@ -1410,6 +1410,38 @@ def test_no_residual_sample_explosion(self): out = self._run(T=1.0, dt=0.1, fps=3, max_step=1e-3) self.assertEqual(len(out.t), 13) # 11 dt-grid + 2 anim_frame boundaries + def test_no_empty_grid_sample_explosion(self): + """An interval with no `k*dt` multiple inside (here the final + `[1/3, 0.34]`) does not dump every integrator step into out.t.""" + out = self._run(T=0.34, dt=0.1, fps=3, max_step=1e-3) + self.assertEqual(len(out.t), 6) # 0, 0.1, 0.2, 0.3, 1/3, 0.34 + + +# --------------------------------------------------------------------------- +class BuildTEvalGridTest(unittest.TestCase): + """Unit tests for `BDSim._build_t_eval_grid`. + + Contract: returns a non-empty `np.ndarray` containing exactly the endpoints + `t0` and `t1` plus any `k*dt` multiples strictly between them, sorted and + deduplicated. + """ + + def test_endpoints_only_when_no_multiple_fits(self): + """`t1 - t0 < dt` and no `k*dt` in `[t0, t1]` → grid is `[t0, t1]`.""" + grid = BDSim._build_t_eval_grid(0.333, 0.34, 0.1) + np.testing.assert_allclose(grid, [0.333, 0.34]) + + def test_aligned_endpoints_no_duplicate(self): + """`t0 == k*dt` and `t1 == (k+n)*dt` → clean dt-aligned sequence, + endpoint inclusion does not introduce duplicates.""" + grid = BDSim._build_t_eval_grid(0.0, 0.3, 0.1) + np.testing.assert_allclose(grid, [0.0, 0.1, 0.2, 0.3]) + + def test_endpoints_with_interior_multiples(self): + """Non-aligned endpoints with `k*dt` points between.""" + grid = BDSim._build_t_eval_grid(0.123, 0.456, 0.1) + np.testing.assert_allclose(grid, [0.123, 0.2, 0.3, 0.4, 0.456]) + if __name__ == "__main__": unittest.main()