Skip to content

Commit 6220755

Browse files
authored
fix(brainpy): repair 18 correctness bugs found in a full-package audit (#868)
A second audited, library-wide bug-fix sweep: 18 confirmed defects across dnn, integrators, encoding, initializers, connectivity, optimizers, dynold STP, offline/online training, object transforms, and runners — each with a co-located regression test. Also removes three legacy numpy_*_test.py files carried over from local master.
1 parent 73a54b3 commit 6220755

35 files changed

Lines changed: 664 additions & 7941 deletions

brainpy/algorithms/offline.py

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,12 @@ def body_fun(a):
173173
par_new2 = par_new - self.learning_rate * grad_w
174174
return i + 1, par_new, par_new2
175175

176-
# Tune parameters for n iterations
177-
r = while_loop(cond_fun, body_fun, (0, w - 1e-8, w))
176+
# Tune parameters for n iterations. Seed ``par_old`` FAR from ``par_new`` so
177+
# the very first convergence check (``not allclose(par_old, par_new)``) passes
178+
# and the body runs at least once. ``w - 1e-8`` is within ``allclose``'s default
179+
# tolerance of ``w``, so the loop exited immediately and returned the untrained
180+
# initial weights (H7, audit 2026-07-08). Matches the ridge path's ``w + 1.``.
181+
r = while_loop(cond_fun, body_fun, (0, w + 1., w))
178182
return r[-1]
179183

180184
def predict(self, W, X):
@@ -415,10 +419,14 @@ def body_fun(a):
415419
# respect to the parameters to minimize the loss
416420
par_new2 = par_new - self.learning_rate * (y_pred - targets).dot(inputs)
417421
else:
418-
gradient = self.sigmoid.grad(inputs.dot(par_new))
419-
diag_grad = bm.zeros((gradient.size, gradient.size))
420-
diag = bm.arange(gradient.size)
421-
diag_grad[diag, diag] = gradient
422+
# Build the diagonal weight matrix with pure JAX ops. The previous
423+
# ``bm.zeros(...)`` + in-place ``diag_grad[diag, diag] = ...`` created a
424+
# ``brainpy.Array`` whose ``__jax_array__`` protocol is invoked during
425+
# ``while_loop`` abstractification, which current JAX rejects
426+
# ("Triggering __jax_array__() during abstractification is no longer
427+
# supported") (M5, audit 2026-07-08).
428+
gradient = bm.as_jax(self.sigmoid.grad(inputs.dot(par_new)))
429+
diag_grad = jnp.diag(gradient)
422430
par_new2 = jnp.linalg.pinv(inputs.T.dot(diag_grad).dot(inputs)).dot(inputs.T).dot(
423431
diag_grad.dot(inputs).dot(par_new) + targets - y_pred)
424432
return i + 1, par_new, par_new2

brainpy/algorithms/offline_test.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,16 @@ def test_gradient_descent_path(self):
9191
w = np.asarray(bm.as_jax(algo(y, x)))
9292
assert np.all(np.isfinite(w))
9393

94+
def test_gradient_descent_actually_fits(self):
95+
# H7 regression: the while_loop was seeded with ``(0, w - 1e-8, w)`` and
96+
# ``allclose(w - 1e-8, w)`` is True, so the convergence check fired
97+
# immediately, the body never ran, and the *untrained* random init weights
98+
# were returned. After the fix the solver must recover the true slope.
99+
x, y = _xy(slope=2.0, n=40)
100+
algo = offline.LinearRegression(gradient_descent=True, max_iter=20000, learning_rate=1e-3)
101+
w = np.asarray(bm.as_jax(algo(y, x))).flatten()
102+
assert np.allclose(w[0], 2.0, atol=1e-2), f'did not fit: w={w}'
103+
94104
def test_predict_and_init_weights(self):
95105
x, y = _xy(slope=2.0)
96106
algo = offline.LinearRegression()
@@ -173,6 +183,24 @@ def test_call_runs_and_separates(self):
173183
acc = np.mean((pred.reshape(-1) > 0.5) == y.reshape(-1))
174184
assert acc >= 0.8
175185

186+
def test_newton_path_runs_and_fits(self):
187+
# M5 regression: the Newton (``gradient_descent=False``) path built a
188+
# ``brainpy.Array`` (``bm.zeros`` + in-place diagonal assignment) inside the
189+
# ``while_loop`` body, which current JAX rejects with
190+
# "Triggering __jax_array__() during abstractification is no longer supported".
191+
rng = np.random.RandomState(2)
192+
X = rng.randn(200, 3).astype(np.float32)
193+
logits = X @ np.array([0.8, -0.5, 0.3], np.float32)
194+
p = 1.0 / (1.0 + np.exp(-logits))
195+
y = (rng.rand(200) < p).astype(np.float32)[:, None] # non-separable (finite MLE)
196+
algo = offline.LogisticRegression(gradient_descent=False, max_iter=100)
197+
w = np.asarray(bm.as_jax(algo(bm.asarray(y), bm.asarray(X)))).reshape(-1)
198+
assert w.shape == (3,)
199+
assert np.all(np.isfinite(w))
200+
# recovers the sign of the true coefficients (0.8, -0.5, 0.3)
201+
assert np.sign(w[0]) == 1
202+
assert np.sign(w[1]) == -1
203+
176204
def test_predict_applies_sigmoid(self):
177205
# ``predict`` itself works in isolation (it does not hit the broken call).
178206
algo = offline.LogisticRegression(max_iter=10)

brainpy/connect/boost_connect_test.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,16 @@ def test_fixedprob_include_self_false_and_pre_ratio():
5858
# no self connections
5959
assert bool(np.all(np.asarray(pre) != np.asarray(post)))
6060
indices, indptr = fp.build_csr()
61-
# with pre_ratio < 1 only the selected pre rows appear in the indptr
62-
# (int(30 * 0.5) = 15 rows -> 16 indptr entries), and counts are monotone.
63-
assert indptr.shape[0] == int(30 * 0.5) + 1
61+
# A valid CSR indptr always spans the FULL pre range (pre_num + 1 entries);
62+
# with pre_ratio < 1 the non-selected pre rows simply have zero out-degree
63+
# (equal consecutive indptr values). The previous truncated indptr of length
64+
# int(pre_num * pre_ratio) + 1 was a malformed CSR (H4, audit 2026-07-08).
65+
assert indptr.shape[0] == 30 + 1
6466
assert bool(np.all(np.diff(np.asarray(indptr)) >= 0))
67+
# indptr is internally consistent: its last entry equals the number of CSR
68+
# indices (edges). (``build_coo`` re-samples per call, so we do not cross-check
69+
# against the separate ``pre`` draw above.)
70+
assert int(np.asarray(indptr)[-1]) == np.asarray(indices).shape[0]
6571
mat = fp.build_mat()
6672
# diagonal cleared
6773
assert not bool(np.any(np.diagonal(np.asarray(mat))))

brainpy/connect/random_conn.py

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,14 @@ def build_coo(self):
133133
return selected_pre_ids.astype(get_idx_type()), selected_post_ids.astype(get_idx_type())
134134

135135
def build_csr(self):
136+
if self.pre_ratio < 1.:
137+
# Only a subset of pre neurons is selected, so the CSR index pointer must
138+
# still span the FULL pre range (non-selected rows have zero out-degree).
139+
# The bespoke ``cumsum`` below produced only ``pre_num_to_select + 1``
140+
# entries, giving a structurally-invalid CSR (indptr length !=
141+
# pre_num + 1). Route the subset case through ``coo2csr`` over the full
142+
# pre range instead (H4, audit 2026-07-08).
143+
return coo2csr(self.build_coo(), self.pre_num)
136144
pre_num_to_select, post_num_to_select, selected_post_ids, pre_ids = self._iii()
137145
pre_nums = jnp.ones(pre_num_to_select) * post_num_to_select
138146
if not self.include_self:
@@ -188,14 +196,24 @@ def __init__(self,
188196

189197
def build_coo(self):
190198
mat_element_num = self.pre_num * self.post_num
191-
if self.num > mat_element_num:
199+
# A float ``num`` in [0, 1] is documented/allowed by ``__init__`` and denotes
200+
# a *fraction* of the all-to-all connection count; coerce it to an absolute
201+
# integer count here since the random routines below need an integer size.
202+
# Previously a float ``num`` reached ``randint``/``choice`` and raised
203+
# ``TypeError: 'float' object cannot be interpreted as an integer``
204+
# (H4, audit 2026-07-08).
205+
if isinstance(self.num, float):
206+
num = int(round(self.num * mat_element_num))
207+
else:
208+
num = self.num
209+
if num > mat_element_num:
192210
raise ConnectorError(f'"num" must be smaller than "all2all num", '
193-
f'but got {self.num} > {mat_element_num}')
211+
f'but got {num} > {mat_element_num}')
194212
if self.allow_multi_conn:
195-
selected_pre_ids = self.rng.randint(0, self.pre_num, (self.num,))
196-
selected_post_ids = self.rng.randint(0, self.post_num, (self.num,))
213+
selected_pre_ids = self.rng.randint(0, self.pre_num, (num,))
214+
selected_post_ids = self.rng.randint(0, self.post_num, (num,))
197215
else:
198-
index = self.rng.choice(mat_element_num, size=(self.num,), replace=False)
216+
index = self.rng.choice(mat_element_num, size=(num,), replace=False)
199217
selected_pre_ids = index // self.post_num
200218
selected_post_ids = index % self.post_num
201219
return selected_pre_ids.astype(get_idx_type()), selected_post_ids.astype(get_idx_type())

brainpy/connect/random_conn_coverage_test.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,11 @@ def test_fixedprob_include_self_false_pure_python(no_numba):
9292
def test_fixedprob_pre_ratio_pure_python(no_numba):
9393
fp = rc.FixedProb(0.5, pre_ratio=0.5, seed=3)((10,), (10,))
9494
indices, indptr = fp.build_csr()
95-
# only int(10 * 0.5) = 5 selected pre rows -> 6 indptr entries
96-
assert indptr.shape[0] == int(10 * 0.5) + 1
95+
# A valid CSR indptr spans the FULL pre range (pre_num + 1 entries); non-selected
96+
# pre rows have zero out-degree. The previous truncated length of
97+
# int(pre_num * pre_ratio) + 1 was a malformed CSR (H4, audit 2026-07-08).
98+
assert indptr.shape[0] == 10 + 1
99+
assert int(np.asarray(indptr)[-1]) == np.asarray(indices).shape[0]
97100

98101

99102
def test_fixedprob_allow_multi_conn_jax_path():

brainpy/connect/random_conn_test.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,44 @@ def test_require_method(self):
4040
mat = conn2.require(10, 20, bp.connect.CONN_MAT)
4141
self.assertTrue(mat.shape == (10, 20))
4242

43+
def test_build_csr_valid_with_pre_ratio(self):
44+
# Regression for H4 (audit 2026-07-08): with ``pre_ratio < 1`` the CSR index
45+
# pointer only spanned the *selected* pre neurons (len == pre_num_to_select+1)
46+
# instead of the full pre range (len == pre_num+1), producing an invalid CSR.
47+
import numpy as np
48+
pre_num = 10
49+
conn = bp.connect.FixedProb(prob=0.5, pre_ratio=0.5, seed=42, allow_multi_conn=True)
50+
conn(pre_size=pre_num, post_size=10)
51+
indices, indptr = conn.build_csr()
52+
indices = np.asarray(indices)
53+
indptr = np.asarray(indptr)
54+
self.assertEqual(len(indptr), pre_num + 1)
55+
self.assertEqual(int(indptr[0]), 0)
56+
self.assertTrue(np.all(np.diff(indptr) >= 0))
57+
self.assertEqual(int(indptr[-1]), len(indices))
58+
self.assertTrue(np.all((indices >= 0) & (indices < 10)))
59+
# Exactly ``int(pre_num * pre_ratio)`` rows carry out-going connections.
60+
nonempty_rows = int(np.sum(np.diff(indptr) > 0))
61+
self.assertEqual(nonempty_rows, int(pre_num * 0.5))
62+
63+
64+
class TestFixedTotalNum(unittest.TestCase):
65+
def test_float_num_is_fraction_of_all2all(self):
66+
# Regression for H4 (audit 2026-07-08): a float ``num`` (documented as allowed
67+
# in [0, 1]) reached ``randint``/``choice`` and raised ``TypeError`` because a
68+
# float cannot be a shape. It now denotes a fraction of the all-to-all count.
69+
conn = bp.connect.FixedTotalNum(num=0.1, seed=1)
70+
conn(pre_size=10, post_size=10) # all2all == 100
71+
pre_ids, post_ids = conn.build_coo()
72+
self.assertEqual(len(pre_ids), 10)
73+
self.assertEqual(len(post_ids), 10)
74+
75+
def test_int_num_unchanged(self):
76+
conn = bp.connect.FixedTotalNum(num=25, seed=1)
77+
conn(pre_size=10, post_size=10)
78+
pre_ids, _ = conn.build_coo()
79+
self.assertEqual(len(pre_ids), 25)
80+
4381

4482
def test_random_fix_pre1():
4583
for num in [0.4, 20]:

brainpy/connect/regular_conn.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,18 @@ def f_connect(pre_id):
192192
strides = jnp.asarray(get_size_length(self.post_size))
193193
pres = jnp.sum(pres * strides, axis=1)
194194
posts = jnp.sum(posts * strides, axis=1)
195+
if self.periodic_boundary:
196+
# On small grids the periodic wrap (``post_ids % sizes``) can map several
197+
# strides onto the same post neuron, producing duplicate ``(pre, post)``
198+
# edges (e.g. a 2x2 GridFour yields each edge twice). De-duplicate while
199+
# preserving order (M1, audit 2026-07-08).
200+
pres_np = np.asarray(pres)
201+
posts_np = np.asarray(posts)
202+
_, uniq_idx = np.unique(np.stack([pres_np, posts_np], axis=1),
203+
axis=0, return_index=True)
204+
uniq_idx = np.sort(uniq_idx)
205+
pres = pres_np[uniq_idx]
206+
posts = posts_np[uniq_idx]
195207
return jnp.asarray(pres, dtype=get_idx_type()), jnp.asarray(posts, dtype=get_idx_type())
196208

197209

brainpy/connect/regular_conn_test.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,3 +116,21 @@ def test_grid_N(self):
116116

117117
print(f'periodic_boundary = {periodic_boundary}, include_self = {include_self}, size = {size}')
118118
self.assertTrue(bp.math.allclose(mat, new_mat))
119+
120+
def test_periodic_boundary_no_duplicate_edges(self):
121+
# Regression for M1 (audit 2026-07-08): on small grids the periodic wrap
122+
# (``post_ids % sizes``) maps several strides onto the same post neuron,
123+
# producing duplicate (pre, post) COO edges (e.g. a 2x2 GridFour yields every
124+
# edge twice). Duplicates are invisible in a boolean matrix but double-count
125+
# weights in COO-based synapses.
126+
for cls, kwargs in [(bp.conn.GridFour, {}),
127+
(bp.conn.GridEight, {}),
128+
(bp.conn.GridN, {'N': 2})]:
129+
for size in [(2, 2), (2, 3), (3, 3)]:
130+
conn = cls(periodic_boundary=True, **kwargs)(size, size)
131+
pre_ids, post_ids = conn.build_coo()
132+
pairs = np.stack([np.asarray(pre_ids), np.asarray(post_ids)], axis=1)
133+
n_unique = len(np.unique(pairs, axis=0))
134+
self.assertEqual(len(pairs), n_unique,
135+
msg=f'{cls.__name__} size={size}: '
136+
f'{len(pairs) - n_unique} duplicate edges')

brainpy/dnn/normalization.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,9 @@ def update(self, x):
557557
if x.shape[-len(self.normalized_shape):] != self.normalized_shape:
558558
raise ValueError(f'Expect the input shape should be (..., {", ".join(map(str, self.normalized_shape))}), '
559559
f'but we got {x.shape}')
560-
axis = tuple(range(0, x.ndim - len(self.normalized_shape)))
560+
# Normalize over the trailing ``normalized_shape`` dimensions (LayerNorm's
561+
# definition), NOT the leading batch/time axes (C1, audit 2026-07-08).
562+
axis = tuple(range(x.ndim - len(self.normalized_shape), x.ndim))
561563
mean = jnp.mean(bm.as_jax(x), axis=axis, keepdims=True)
562564
variance = jnp.var(bm.as_jax(x), axis=axis, keepdims=True)
563565
inv = lax.rsqrt(variance + lax.convert_element_type(self.epsilon, x.dtype))

brainpy/dnn/normalization_test.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,30 @@ def test_BatchNorm_batch_is_biased_normalized(self):
118118
self.assertTrue(bool(jnp.allclose(out.mean(axis=(0, 1)), 0.0, atol=1e-5)))
119119
self.assertTrue(bool(jnp.allclose(out.var(axis=(0, 1)), 1.0, atol=1e-4)))
120120

121+
def test_LayerNorm_normalizes_over_trailing_axes(self):
122+
# Regression for C1 (audit 2026-07-08): ``LayerNorm`` must normalize over the
123+
# trailing ``normalized_shape`` dimensions, not the leading (batch/time) axes.
124+
# With the bug it reduced over ``range(0, ndim - len(normalized_shape))`` and
125+
# produced results ~1.7 away from the reference.
126+
import jax.numpy as jnp
127+
bm.random.seed(2718)
128+
normalized_shape = (5, 10)
129+
net = bp.dnn.LayerNorm(normalized_shape, elementwise_affine=False, mode=bm.training_mode)
130+
x = bm.as_jax(bm.random.randn(4, 3, 5, 10) * 3.0 + 2.0)
131+
out = bm.as_jax(net(x))
132+
133+
# Reference: normalize each (5, 10) block independently over its last 2 dims.
134+
axes = (-2, -1)
135+
mean = jnp.mean(x, axis=axes, keepdims=True)
136+
var = jnp.var(x, axis=axes, keepdims=True)
137+
ref = (x - mean) / jnp.sqrt(var + 1e-5)
138+
self.assertTrue(bool(jnp.allclose(out, ref, atol=1e-4)),
139+
msg=f'max|out-ref|={float(jnp.max(jnp.abs(out - ref)))}')
140+
141+
# Each normalized block must be (approximately) zero-mean / unit-variance.
142+
self.assertTrue(bool(jnp.allclose(jnp.mean(out, axis=axes), 0.0, atol=1e-4)))
143+
self.assertTrue(bool(jnp.allclose(jnp.var(out, axis=axes), 1.0, atol=1e-3)))
144+
121145

122146
if __name__ == '__main__':
123147
absltest.main()

0 commit comments

Comments
 (0)