Skip to content

Commit 3204311

Browse files
authored
fix(core): DSRunner memory_efficient output/monitors; eager check bound validation (#851)
fix(core): DSRunner memory_efficient output/monitor axes; eager check.is_float/is_integer bounds - DSRunner(memory_efficient=True) returned None instead of model outputs (list.append inside tree_map); now accumulates and stacks per-step outputs along a leading time axis (Critical) - memory_efficient monitors came out time-major vs the standard path's batch-major for BatchingMode; moveaxis to match (High) - check.is_float/is_integer min_bound/max_bound never raised eagerly (pure_callback swallowed the raise for concrete predicates); add a concrete-predicate fast path that raises directly, keeping the deferred cond/callback path under tracing (High) Findings recorded in docs/issues-found-20260619-toplevel-glue.md
1 parent 3714692 commit 3204311

6 files changed

Lines changed: 527 additions & 34 deletions

File tree

brainpy/check.py

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -620,6 +620,16 @@ def jit_error(pred, err_fun, err_arg=None):
620620
The arguments which passed into `err_f`.
621621
"""
622622
from brainpy.math.interoperability import as_jax
623+
624+
# Fast path for a concrete (non-traced) predicate. ``jax.pure_callback`` only
625+
# surfaces its raised exception when the staged computation is *executed*; for
626+
# an eager, concrete predicate the callback never runs synchronously, so the
627+
# error would be silently swallowed. Evaluate and raise directly instead.
628+
if not isinstance(pred, jax.core.Tracer):
629+
if bool(np.asarray(pred)):
630+
err_fun(err_arg)
631+
return
632+
623633
partial(_cond, err_fun)(as_jax(pred), err_arg)
624634

625635

@@ -641,7 +651,16 @@ def jit_error_checking_no_args(pred: bool, err: Exception):
641651

642652
assert isinstance(err, Exception), 'Must be instance of Exception.'
643653

644-
def true_err_fun(arg, transforms):
654+
# Fast path for a concrete (non-traced) predicate. The ``jax.pure_callback``
655+
# below only raises when the staged computation is *executed*; eagerly the
656+
# exception is never surfaced synchronously, so an out-of-bound value would
657+
# be silently accepted (e.g. ``is_float(2.0, max_bound=1.0)``). Raise here.
658+
if not isinstance(pred, jax.core.Tracer):
659+
if bool(np.asarray(pred)):
660+
raise err
661+
return
662+
663+
def true_err_fun(*args):
645664
raise err
646665

647666
cond(unvmap(as_jax(pred)),

brainpy/check_coverage_test.py

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
``is_all_vars``, ``is_all_objs``), ``serialize_kwargs``, and the JIT error
1515
helpers (``jit_error``, ``jit_error_checking``, ``jit_error_checking_no_args``).
1616
"""
17+
import jax
1718
import numpy as np
1819
import pytest
1920

@@ -306,18 +307,18 @@ def test_min_bound_ok(self):
306307
assert checking.is_float(5.0, min_bound=1.0) == 5.0
307308

308309
def test_min_bound_branch(self):
309-
# NOTE: min/max bound checks route through jit_error_checking_no_args
310-
# which uses jax.pure_callback. Eagerly the side-effect raise is NOT
311-
# propagated, so an out-of-bound value returns rather than raising.
312-
# We exercise the branch (line coverage) and pin the no-raise behavior.
313-
assert checking.is_float(0.5, min_bound=1.0, name='v') == 0.5
310+
# P14-H2: eager out-of-bound checks now raise (previously the
311+
# jit_error_checking_no_args pure_callback path silently accepted them).
312+
with pytest.raises(Exception):
313+
checking.is_float(0.5, min_bound=1.0, name='v')
314314

315315
def test_max_bound_ok(self):
316316
assert checking.is_float(5.0, max_bound=10.0) == 5.0
317317

318318
def test_max_bound_branch(self):
319-
# NOTE: see test_min_bound_branch -- eager pure_callback does not raise.
320-
assert checking.is_float(20.0, max_bound=10.0, name='v') == 20.0
319+
# P14-H2: eager out-of-bound checks now raise.
320+
with pytest.raises(Exception):
321+
checking.is_float(20.0, max_bound=10.0, name='v')
321322

322323

323324
# --------------------------------------------------------------------------- #
@@ -354,16 +355,17 @@ def test_min_bound_ok(self):
354355
assert checking.is_integer(5, min_bound=1) == 5
355356

356357
def test_min_bound_branch(self):
357-
# NOTE: bound check goes through jit_error_checking_no_args (pure_callback);
358-
# eager evaluation does not propagate the raise -- value is returned.
359-
assert checking.is_integer(0, min_bound=1, name='v') == 0
358+
# P14-H2: eager out-of-bound checks now raise.
359+
with pytest.raises(Exception):
360+
checking.is_integer(0, min_bound=1, name='v')
360361

361362
def test_max_bound_ok(self):
362363
assert checking.is_integer(5, max_bound=10) == 5
363364

364365
def test_max_bound_branch(self):
365-
# NOTE: see test_min_bound_branch.
366-
assert checking.is_integer(20, max_bound=10, name='v') == 20
366+
# P14-H2: eager out-of-bound checks now raise.
367+
with pytest.raises(Exception):
368+
checking.is_integer(20, max_bound=10, name='v')
367369

368370

369371
# --------------------------------------------------------------------------- #
@@ -533,18 +535,26 @@ def test_all_objs_bad(self):
533535
# jit error helpers
534536
# --------------------------------------------------------------------------- #
535537
class TestJitErrors:
536-
# NOTE: these helpers wrap ``jax.lax.cond`` + ``jax.pure_callback``. When the
537-
# predicate is True the error callback executes, but eagerly the raised
538-
# exception is NOT propagated synchronously to the caller (a known quirk of
539-
# pure_callback used for in-jit error signalling). We therefore exercise the
540-
# True/False branches for line coverage and assert they run without crashing.
538+
# P14-H2: for a *concrete* predicate these helpers now raise synchronously
539+
# (previously the ``jax.pure_callback`` path silently swallowed the raise).
540+
# Under tracing they keep the deferred ``cond`` + ``pure_callback`` path.
541541
def test_no_args_pred_false(self):
542-
# pred False -> false branch only
542+
# pred False -> false branch only, no raise
543543
checking.jit_error_checking_no_args(False, ValueError('boom'))
544544

545545
def test_no_args_pred_true(self):
546-
# exercises the true branch (callback path), no propagated raise eagerly
547-
checking.jit_error_checking_no_args(True, ValueError('boom'))
546+
# concrete True -> raises synchronously
547+
with pytest.raises(ValueError):
548+
checking.jit_error_checking_no_args(True, ValueError('boom'))
549+
550+
def test_no_args_under_jit(self):
551+
# tracer predicate -> deferred, no raise at trace time
552+
@jax.jit
553+
def f(x):
554+
checking.jit_error_checking_no_args(x > 1.0, ValueError('boom'))
555+
return x
556+
557+
assert float(f(0.0)) == 0.0
548558

549559
def test_no_args_bad_err(self):
550560
with pytest.raises(AssertionError):
@@ -561,15 +571,17 @@ def test_jit_error_true(self):
561571
def err_fun(arg):
562572
raise ValueError('boom')
563573

564-
# exercises the true branch + _err_jit_true_branch single-array path
565-
checking.jit_error(True, err_fun, bm.as_jax(bm.zeros(2)))
574+
# concrete True -> raises synchronously
575+
with pytest.raises(ValueError):
576+
checking.jit_error(True, err_fun, bm.as_jax(bm.zeros(2)))
566577

567578
def test_jit_error_true_tuple_arg(self):
568579
def err_fun(arg):
569580
raise ValueError('boom')
570581

571-
# exercises the tuple/list branch of _err_jit_true_branch
572-
checking.jit_error(True, err_fun, (bm.as_jax(bm.zeros(2)), bm.as_jax(bm.ones(3))))
582+
# concrete True with a tuple err_arg -> raises synchronously
583+
with pytest.raises(ValueError):
584+
checking.jit_error(True, err_fun, (bm.as_jax(bm.zeros(2)), bm.as_jax(bm.ones(3))))
573585

574586
def test_alias(self):
575587
assert checking.jit_error_checking is checking.jit_error

brainpy/check_test.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,62 @@
1515
# ==============================================================================
1616
import unittest
1717

18+
import jax
19+
1820
from brainpy import check as checking
1921

2022

23+
class TestBoundChecks(unittest.TestCase):
24+
"""Regression tests for P14-H2: eager bound checks must actually raise.
25+
26+
``is_float``/``is_integer`` route their ``min_bound``/``max_bound`` checks
27+
through ``jit_error_checking_no_args``. The previous implementation used a
28+
``jax.pure_callback`` whose raise never propagated for a *concrete* (eager)
29+
predicate, so out-of-bound values were silently accepted.
30+
"""
31+
32+
def test_is_float_min_bound_raises(self):
33+
with self.assertRaises(Exception):
34+
checking.is_float(0.5, 'v', min_bound=1.0)
35+
36+
def test_is_float_max_bound_raises(self):
37+
with self.assertRaises(Exception):
38+
checking.is_float(20.0, 'v', max_bound=10.0)
39+
40+
def test_is_float_within_bounds_ok(self):
41+
self.assertEqual(checking.is_float(5.0, 'v', min_bound=1.0, max_bound=10.0), 5.0)
42+
43+
def test_is_integer_min_bound_raises(self):
44+
with self.assertRaises(Exception):
45+
checking.is_integer(0, 'v', min_bound=1)
46+
47+
def test_is_integer_max_bound_raises(self):
48+
with self.assertRaises(Exception):
49+
checking.is_integer(20, 'v', max_bound=10)
50+
51+
def test_is_integer_within_bounds_ok(self):
52+
self.assertEqual(checking.is_integer(5, 'v', min_bound=1, max_bound=10), 5)
53+
54+
def test_no_args_concrete_true_raises(self):
55+
with self.assertRaises(ValueError):
56+
checking.jit_error_checking_no_args(True, ValueError('boom'))
57+
58+
def test_no_args_concrete_false_ok(self):
59+
# must not raise
60+
checking.jit_error_checking_no_args(False, ValueError('boom'))
61+
62+
def test_no_args_under_jit_does_not_raise_at_trace(self):
63+
# When the predicate is a tracer (inside jit) the check must NOT raise
64+
# at trace time; it stays a deferred in-jit error signal.
65+
@jax.jit
66+
def f(x):
67+
checking.jit_error_checking_no_args(x > 1.0, ValueError('boom'))
68+
return x
69+
70+
# tracing/compiling with a value that does not trip the predicate runs fine
71+
self.assertEqual(float(f(0.0)), 0.0)
72+
73+
2174
class TestUtils(unittest.TestCase):
2275
def test_check_shape(self):
2376
all_shapes = [

brainpy/dyn_runner_test.py

Lines changed: 150 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,26 @@
1515
# ==============================================================================
1616
import unittest
1717

18+
import numpy as np
19+
1820
import brainpy as bp
1921
import brainpy.math as bm
2022

23+
# Capture the default ``dt`` before any test mutates it. ``DSRunner(dt=...)``
24+
# permanently writes ``dt`` into the global brainstate environment (via
25+
# ``share.save(dt=...)``), which otherwise leaks ``dt`` into later test files
26+
# (e.g. delay tests that assume the default ``dt=0.1``).
27+
_DEFAULT_DT = bm.get_dt()
28+
29+
30+
class _DtRestoreMixin:
31+
"""Restore the global ``dt`` after each test that runs a ``DSRunner``."""
32+
33+
def tearDown(self):
34+
bm.set_dt(_DEFAULT_DT)
2135

22-
class TestDSRunner(unittest.TestCase):
36+
37+
class TestDSRunner(_DtRestoreMixin, unittest.TestCase):
2338
def test1(self):
2439
class ExampleDS(bp.DynamicalSystem):
2540
def __init__(self):
@@ -89,5 +104,137 @@ def __init__(self, scale=1.0, method='exp_auto'):
89104
inputs=[(net.E.input, 20.), (net.I.input, 20.)], jit=False).run(0.2)
90105

91106

92-
class TestMemoryEfficient(unittest.TestCase):
93-
pass
107+
class TestMemoryEfficient(_DtRestoreMixin, unittest.TestCase):
108+
"""Regression tests for ``DSRunner(memory_efficient=True)`` (P14-C1).
109+
110+
The memory-efficient path collects monitors via a host-side callback and
111+
stacks the model's ``update()`` outputs manually. A previous bug used
112+
``list.append`` inside ``tree_map`` (which returns ``None``) so ``.run()``
113+
silently returned ``None`` instead of the time-stacked outputs.
114+
"""
115+
116+
def _scalar_ds(self):
117+
class ExampleDS(bp.DynamicalSystem):
118+
def __init__(self):
119+
super().__init__()
120+
self.i = bm.Variable(bm.zeros(1))
121+
122+
def update(self):
123+
self.i += 1.
124+
return self.i.value
125+
126+
return ExampleDS
127+
128+
def test_output_matches_normal_scalar(self):
129+
DS = self._scalar_ds()
130+
131+
out_normal = bp.DSRunner(DS(), dt=1., progress_bar=False,
132+
memory_efficient=False).run(5.)
133+
out_mem = bp.DSRunner(DS(), dt=1., progress_bar=False,
134+
memory_efficient=True).run(5.)
135+
136+
out_normal = np.asarray(out_normal)
137+
out_mem = np.asarray(out_mem)
138+
# the memory-efficient output must not be lost
139+
self.assertIsNotNone(out_mem.dtype)
140+
self.assertEqual(out_normal.shape, out_mem.shape)
141+
self.assertTrue(np.allclose(out_normal, out_mem))
142+
self.assertTrue(np.allclose(out_mem.ravel(), [1., 2., 3., 4., 5.]))
143+
144+
def test_output_matches_normal_pytree(self):
145+
# the update returns a dict (a non-trivial pytree of outputs)
146+
class ExampleDS(bp.DynamicalSystem):
147+
def __init__(self):
148+
super().__init__()
149+
self.i = bm.Variable(bm.zeros(2))
150+
151+
def update(self):
152+
self.i += 1.
153+
return {'a': self.i.value, 'b': self.i.value * 2.}
154+
155+
out_normal = bp.DSRunner(ExampleDS(), dt=1., progress_bar=False,
156+
memory_efficient=False).run(4.)
157+
out_mem = bp.DSRunner(ExampleDS(), dt=1., progress_bar=False,
158+
memory_efficient=True).run(4.)
159+
160+
for key in ('a', 'b'):
161+
a = np.asarray(out_normal[key])
162+
b = np.asarray(out_mem[key])
163+
self.assertEqual(a.shape, b.shape)
164+
self.assertEqual(a.shape, (4, 2))
165+
self.assertTrue(np.allclose(a, b))
166+
167+
def test_monitors_still_match(self):
168+
DS = self._scalar_ds()
169+
r_n = bp.DSRunner(DS(), dt=1., monitors=['i'], progress_bar=False,
170+
memory_efficient=False)
171+
r_n.run(5.)
172+
r_m = bp.DSRunner(DS(), dt=1., monitors=['i'], progress_bar=False,
173+
memory_efficient=True)
174+
r_m.run(5.)
175+
self.assertTrue(np.allclose(np.asarray(r_n.mon['i']),
176+
np.asarray(r_m.mon['i'])))
177+
self.assertTrue(np.allclose(np.asarray(r_n.mon['ts']),
178+
np.asarray(r_m.mon['ts'])))
179+
180+
def test_output_none_when_update_returns_none(self):
181+
# an ``update()`` with no explicit return must give ``None`` in both
182+
# paths (the all-``None`` pytree collapses to ``None``), not crash.
183+
class DS(bp.DynamicalSystem):
184+
def __init__(self):
185+
super().__init__()
186+
self.i = bm.Variable(bm.zeros(1))
187+
188+
def update(self):
189+
self.i += 1. # returns None
190+
191+
out_n = bp.DSRunner(DS(), dt=1., monitors=['i'], progress_bar=False,
192+
memory_efficient=False).run(5.)
193+
r_m = bp.DSRunner(DS(), dt=1., monitors=['i'], progress_bar=False,
194+
memory_efficient=True)
195+
out_m = r_m.run(5.)
196+
self.assertIsNone(out_n)
197+
self.assertIsNone(out_m)
198+
self.assertTrue(np.allclose(np.asarray(r_m.mon['i']).ravel(),
199+
[1., 2., 3., 4., 5.]))
200+
201+
def test_batched_monitor_axis_matches_normal(self):
202+
"""P14-H1: for BatchingMode + data_first_axis='B' (the default for
203+
batched models) the memory-efficient monitors must come out batch-major
204+
``(B, T, ...)``, identical to the standard path. The bug returned
205+
time-major ``(T, B, ...)`` only for the memory-efficient path."""
206+
207+
class Net(bp.DynamicalSystem):
208+
def __init__(self):
209+
super().__init__(mode=bm.BatchingMode(4))
210+
self.n = bp.dyn.LifRef(3, mode=bm.BatchingMode(4))
211+
212+
def update(self, inp):
213+
self.n(inp)
214+
return self.n.V.value
215+
216+
inp = bm.ones((4, 8, 3)) * 2.0 # (batch, time, features), data_first_axis='B'
217+
218+
bm.random.seed(0)
219+
net = Net(); net.reset(4)
220+
r_n = bp.DSRunner(net, monitors=['n.V'], memory_efficient=False,
221+
progress_bar=False)
222+
out_n = r_n.run(inputs=inp)
223+
224+
bm.random.seed(0)
225+
net2 = Net(); net2.reset(4)
226+
r_m = bp.DSRunner(net2, monitors=['n.V'], memory_efficient=True,
227+
progress_bar=False)
228+
out_m = r_m.run(inputs=inp)
229+
230+
# outputs are batch-major in both paths
231+
self.assertEqual(np.asarray(out_n).shape, np.asarray(out_m).shape)
232+
self.assertEqual(np.asarray(out_m).shape, (4, 8, 3))
233+
self.assertTrue(np.allclose(np.asarray(out_n), np.asarray(out_m)))
234+
235+
# monitors must share the same (batch, time, features) layout
236+
self.assertEqual(np.asarray(r_n.mon['n.V']).shape,
237+
np.asarray(r_m.mon['n.V']).shape)
238+
self.assertEqual(np.asarray(r_m.mon['n.V']).shape, (4, 8, 3))
239+
self.assertTrue(np.allclose(np.asarray(r_n.mon['n.V']),
240+
np.asarray(r_m.mon['n.V'])))

0 commit comments

Comments
 (0)