Skip to content

Commit 3e29c8e

Browse files
authored
fix(inputs,algorithms,connect): deprecation aliases, regression fits, CSR guard, dtype (#853)
fix(inputs,algorithms,connect): deprecation aliases, regression fits, CSR guard, dtype/off-by-one - spike_current/ramp_current deprecation aliases forwarded to constant_input with wrong kwargs (crash); forward to spike_input/ramp_input (Critical) - LogisticRegression.call crashed (flattened targets then read .shape[1]); init a 1-D parameter vector (Critical) - ElasticNetRegression trained with default add_bias=True while predict used self.add_bias -> feature mismatch crash; pass add_bias=self.add_bias (High) - CSRConn.build_csr guard "pre_num != pre_num" was a tautology; compare against inptr.size-1 and raise on inconsistent pre_size (High) - connect.coo2csr scattered int counts into a uint32 buffer (int32->uint32 warning, future JAX error); use get_idx_type() (Medium) - polynomial_features allocated one extra always-zero column (off-by-one) (Medium) Findings recorded in docs/issues-found-20260619-small-modules.md
1 parent 4ec07f4 commit 3e29c8e

11 files changed

Lines changed: 341 additions & 46 deletions

brainpy/algorithms/offline.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -385,8 +385,11 @@ def call(self, targets, inputs, outputs=None) -> ArrayType:
385385
raise ValueError(f'Target must be a scalar, but got multiple variables: {targets.shape}. ')
386386
targets = targets.flatten()
387387

388-
# initialize parameters
389-
param = self.init_weights(inputs.shape[1], targets.shape[1])
388+
# Initialize a 1-D parameter vector to match the flattened 1-D
389+
# ``targets`` used below. (``init_weights`` returns a 2-D ``(n_features,
390+
# n_out)`` array; reading ``targets.shape[1]`` after the flatten raised
391+
# IndexError, so request a single output and squeeze to 1-D.)
392+
param = self.init_weights(inputs.shape[1], 1).flatten()
390393

391394
def cond_fun(a):
392395
i, par_old, par_new = a
@@ -538,8 +541,9 @@ def call(self, targets, inputs, outputs=None):
538541
# checking
539542
inputs = _check_data_2d_atls(bm.as_jax(inputs))
540543
targets = _check_data_2d_atls(bm.as_jax(targets))
541-
# solving
542-
inputs = normalize(polynomial_features(inputs, degree=self.degree))
544+
# solving. ``add_bias`` must match ``predict`` so the fitted weight
545+
# length equals the feature width seen at prediction time.
546+
inputs = normalize(polynomial_features(inputs, degree=self.degree, add_bias=self.add_bias))
543547
return super(ElasticNetRegression, self).gradient_descent_solve(targets, inputs)
544548

545549
def predict(self, W, X):

brainpy/algorithms/offline_test.py

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -154,18 +154,24 @@ def test_fit_and_predict(self):
154154

155155

156156
class TestLogisticRegression:
157-
def test_call_is_currently_broken(self):
158-
# NOTE (defect): LogisticRegression.call flattens ``targets`` to 1-D
159-
# (offline.py line ~386) and then immediately reads ``targets.shape[1]``
160-
# at line ~389, which raises IndexError. The closed-form (non gradient
161-
# descent) branch is also unreachable because of this. The fit path is
162-
# therefore broken for both gradient_descent=True and =False.
157+
def test_call_runs_and_separates(self):
158+
# P16-C2 (was test_call_is_currently_broken): ``LogisticRegression.call``
159+
# used to crash with IndexError because it flattened ``targets`` to 1-D
160+
# and then read ``targets.shape[1]``. After the fix it must run and learn
161+
# a usable separator on a trivially separable problem.
163162
rng = np.random.RandomState(1)
164163
x = rng.uniform(-1, 1, size=(30, 2)).astype(np.float32)
165164
y = (x[:, :1] > 0).astype(np.float32)
166-
algo = offline.LogisticRegression(max_iter=50, learning_rate=0.1)
167-
with pytest.raises(IndexError):
168-
algo(bm.asarray(y), bm.asarray(x))
165+
algo = offline.LogisticRegression(max_iter=200, learning_rate=0.5)
166+
w = algo(bm.asarray(y), bm.asarray(x))
167+
w = np.asarray(bm.as_jax(w))
168+
# one weight per input feature (1-D parameter vector)
169+
assert w.reshape(-1).shape == (2,)
170+
assert np.all(np.isfinite(w))
171+
# predictions should mostly agree with the (separable) labels
172+
pred = np.asarray(bm.as_jax(algo.predict(bm.asarray(w), bm.asarray(x))))
173+
acc = np.mean((pred.reshape(-1) > 0.5) == y.reshape(-1))
174+
assert acc >= 0.8
169175

170176
def test_predict_applies_sigmoid(self):
171177
# ``predict`` itself works in isolation (it does not hit the broken call).
@@ -209,19 +215,29 @@ def test_elastic_net_regression_fit(self):
209215
w = np.asarray(bm.as_jax(algo(y, x)))
210216
assert np.all(np.isfinite(w))
211217

212-
def test_elastic_net_predict_bias_mismatch_is_broken(self):
213-
# NOTE (defect): ElasticNetRegression.call builds features with
214-
# ``polynomial_features(inputs, degree=self.degree)`` which defaults to
215-
# add_bias=True, while ``predict`` calls it with add_bias=self.add_bias
216-
# (default False). The resulting feature width differs from the fitted
217-
# weight length, so predicting on freshly-built features raises a
218-
# shape-mismatch TypeError from jnp.dot.
218+
def test_elastic_net_train_predict_consistent(self):
219+
# P16-H1 (was test_elastic_net_predict_bias_mismatch_is_broken):
220+
# ``call`` used to build features with the default add_bias=True while
221+
# ``predict`` used add_bias=self.add_bias (default False), giving a
222+
# train/predict feature-count mismatch that crashed jnp.dot. After the
223+
# fix, training and prediction must use identical feature construction.
219224
x, y = _xy(slope=2.0)
220225
algo = offline.ElasticNetRegression(alpha=0.01, degree=2, l1_ratio=0.5,
221226
max_iter=50, learning_rate=0.001)
222227
w = bm.asarray(np.asarray(bm.as_jax(algo(y, x))))
223-
with pytest.raises(TypeError):
224-
algo.predict(w, x)
228+
pred = np.asarray(bm.as_jax(algo.predict(w, x)))
229+
assert pred.shape[0] == np.asarray(bm.as_jax(x)).shape[0]
230+
assert np.all(np.isfinite(pred))
231+
232+
def test_elastic_net_add_bias_true_consistent(self):
233+
# The fix must also hold when add_bias=True is requested explicitly.
234+
x, y = _xy(slope=2.0)
235+
algo = offline.ElasticNetRegression(alpha=0.01, degree=2, l1_ratio=0.5,
236+
add_bias=True, max_iter=50,
237+
learning_rate=0.001)
238+
w = bm.asarray(np.asarray(bm.as_jax(algo(y, x))))
239+
pred = np.asarray(bm.as_jax(algo.predict(w, x)))
240+
assert np.all(np.isfinite(pred))
225241

226242

227243
class TestRegistry:

brainpy/algorithms/utils.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,11 @@ def polynomial_features(X, degree: int, add_bias: bool = True):
106106
return bm.insert(X, 0, 1, axis=1) if add_bias else X
107107
if add_bias:
108108
n_features += 1
109-
X_new = bm.zeros((n_samples, 1 + n_features + len(combinations)))
109+
# ``n_features`` already accounts for the bias slot (when ``add_bias``); the
110+
# design matrix is exactly the (bias +) original features plus the degree>=2
111+
# interaction terms. The previous extra leading ``1 +`` left a dead all-zero
112+
# trailing column and over-counted the feature dimension by one.
113+
X_new = bm.zeros((n_samples, n_features + len(combinations)))
110114
if add_bias:
111115
X_new[:, 0] = 1
112116
X_new[:, 1:n_features] = X

brainpy/algorithms/utils_test.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -97,25 +97,25 @@ def test_degree1_shortcircuit_without_bias(self):
9797
def test_degree2_with_bias(self):
9898
X = bm.asarray([[2.0, 3.0]])
9999
out = np.asarray(bm.as_jax(utils.polynomial_features(X, degree=2, add_bias=True)))
100-
# X_new has shape (n_samples, 1 + n_features + len(combinations)).
101-
# With add_bias, n_features is bumped to 3 (2 + 1), combos == 3, so the
102-
# allocated width is 1 + 3 + 3 == 7. NOTE: the leading "1 +" plus the
103-
# bumped n_features leaves one all-zero trailing column unused, i.e. the
104-
# output is wider than the mathematically expected 6 columns.
105-
assert out.shape == (1, 7)
100+
# P16-M2: width is exactly 1 bias + 2 linear + 3 interaction == 6
101+
# (previously a dead all-zero trailing column made it 7).
102+
assert out.shape == (1, 6)
106103
assert out[0, 0] == 1.0
107104
# the linear features should appear
108105
assert 2.0 in out[0] and 3.0 in out[0]
109106
# interactions: 4 (=2^2), 6 (=2*3), 9 (=3^2)
110107
for v in (4.0, 6.0, 9.0):
111108
assert np.any(np.isclose(out[0], v))
109+
# no dead all-zero column anymore
110+
assert not np.any(np.all(out == 0, axis=0))
112111

113112
def test_degree2_without_bias(self):
114113
X = bm.asarray([[2.0, 3.0]])
115114
out = np.asarray(bm.as_jax(utils.polynomial_features(X, degree=2, add_bias=False)))
116-
# 1 + 2 linear + 3 interaction -> 6 cols (again one extra leading column;
117-
# see NOTE in test_degree2_with_bias).
118-
assert out.shape == (1, 6)
115+
# P16-M2: 2 linear + 3 interaction -> 5 cols (previously 6 with a dead
116+
# leading allocation slot).
117+
assert out.shape == (1, 5)
118+
assert not np.any(np.all(out == 0, axis=0))
119119

120120

121121
class TestNormalize:

brainpy/connect/base.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -689,15 +689,20 @@ def coo2csr(coo, num_pre):
689689
post_ids = post_ids[sort_ids]
690690
indices = post_ids
691691
unique_pre_ids, pre_count = onp.unique(pre_ids, return_counts=True)
692-
final_pre_count = onp.zeros(num_pre, dtype=jnp.uint32)
692+
# Use the connection index dtype (not uint32) so the assignment below does
693+
# not trigger an int->uint scatter dtype mismatch.
694+
final_pre_count = onp.zeros(num_pre, dtype=get_idx_type())
693695
final_pre_count[unique_pre_ids] = pre_count
694696
else:
695697
sort_ids = onp.argsort(bm.as_jax(pre_ids))
696698
post_ids = bm.as_jax(post_ids)
697699
post_ids = post_ids[sort_ids]
698700
indices = post_ids
699701
unique_pre_ids, pre_count = jnp.unique(pre_ids, return_counts=True)
700-
final_pre_count = bm.zeros(num_pre, dtype=jnp.uint32)
702+
# Use the connection index dtype (not uint32) so the in-place update below
703+
# does not trigger an int->uint scatter dtype mismatch (FutureWarning that
704+
# becomes an error in future JAX releases).
705+
final_pre_count = bm.zeros(num_pre, dtype=get_idx_type())
701706
final_pre_count[unique_pre_ids] = pre_count
702707
final_pre_count = bm.as_jax(final_pre_count)
703708
indptr = final_pre_count.cumsum()

brainpy/connect/custom_conn.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ def __init__(self, indices, inptr, **kwargs):
100100
self.max_post = self.indices.max()
101101

102102
def build_csr(self):
103-
if self.pre_num != self.pre_num:
103+
if self.pre_num != self.inptr.size - 1:
104104
raise ConnectorError(f'(pre_size, post_size) is inconsistent with '
105105
f'the shape of the sparse matrix.')
106106
if self.post_num <= self.max_post:

brainpy/connect/custom_conn_test.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,45 @@ def test_MatConn2(self):
6161
conn(pre_size=5, post_size=1)
6262

6363

64+
class TestCSRConn(TestCase):
65+
def _csr(self):
66+
# 3 pre-synaptic neurons (indptr has 4 entries), max post id == 2
67+
indices = np.array([0, 1, 2, 0, 1], dtype=np.int32)
68+
indptr = np.array([0, 2, 3, 5], dtype=np.int32)
69+
return indices, indptr
70+
71+
def test_csrconn_consistent_ok(self):
72+
# P16-H2: a CSRConn whose declared pre_size matches the indptr length
73+
# must build without error.
74+
indices, indptr = self._csr()
75+
conn = bp.conn.CSRConn(indices, indptr)
76+
ind, ip = conn.require(3, 3, 'csr')
77+
assert np.array_equal(np.asarray(ind), indices)
78+
assert np.array_equal(np.asarray(ip), indptr)
79+
80+
def test_csrconn_inconsistent_pre_num_raises(self):
81+
# P16-H2: previously the guard ``self.pre_num != self.pre_num`` was a
82+
# tautology (always False), so an inconsistent pre_size silently produced
83+
# a malformed CSR. It must now raise.
84+
indices, indptr = self._csr() # indptr implies 3 pre
85+
conn = bp.conn.CSRConn(indices, indptr)
86+
with pytest.raises(bp.errors.ConnectorError):
87+
conn.require(5, 3, 'csr') # pre=5 inconsistent with indptr (3)
88+
89+
def test_coo2csr_no_dtype_warning(self):
90+
# P16-M1: coo2csr must not emit the int32->uint32 scatter FutureWarning
91+
# (which is slated to become an error in future JAX).
92+
import warnings
93+
import jax.numpy as jnp
94+
from brainpy.connect.base import coo2csr
95+
pre = jnp.array([0, 0, 1, 2, 2, 2])
96+
post = jnp.array([1, 2, 0, 0, 1, 2])
97+
with warnings.catch_warnings():
98+
warnings.simplefilter('error', FutureWarning)
99+
ind, indptr = coo2csr((pre, post), 3)
100+
assert np.array_equal(np.asarray(indptr), np.array([0, 2, 3, 6]))
101+
102+
64103
class TestSparseMatConn(TestCase):
65104
def test_sparseMatConn(self):
66105
conn_mat = np.random.randint(2, size=(5, 3), dtype=bp.math.bool_)

brainpy/inputs/currents.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ def spike_current(*args, **kwargs):
150150
warnings.warn('Please use "brainpy.inputs.spike_input()" instead. '
151151
'"brainpy.inputs.spike_current()" is deprecated since version 2.1.13.',
152152
DeprecationWarning)
153-
return constant_input(*args, **kwargs)
153+
return spike_input(*args, **kwargs)
154154

155155

156156
def ramp_input(c_start, c_end, duration, t_start=0, t_end=None, dt=None):
@@ -189,7 +189,7 @@ def ramp_current(*args, **kwargs):
189189
warnings.warn('Please use "brainpy.inputs.ramp_input()" instead. '
190190
'"brainpy.inputs.ramp_current()" is deprecated since version 2.1.13.',
191191
DeprecationWarning)
192-
return constant_input(*args, **kwargs)
192+
return ramp_input(*args, **kwargs)
193193

194194

195195
def wiener_process(duration, dt=None, n=1, t_start=0., t_end=None, seed=None):

brainpy/inputs/currents_coverage_test.py

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,25 +43,27 @@ def test_constant_current_warns_and_delegates(self):
4343
self.assertEqual(duration, 200)
4444

4545
def test_spike_current_warns(self):
46-
# NOTE: ``spike_current`` is documented as a spike-input shim but its
47-
# body actually delegates to ``constant_input`` (copy/paste defect in
48-
# the deprecation wrapper), so it must be fed constant_input-style
49-
# ``[(value, duration), ...]`` pairs rather than spike arguments.
46+
# P16-C1: ``spike_current`` now correctly delegates to ``spike_input``
47+
# (it previously delegated to ``constant_input`` and crashed on spike
48+
# arguments). It must warn AND accept spike-style arguments.
49+
kwargs = dict(sp_times=[10, 20, 30], sp_lens=1., sp_sizes=0.5, duration=40.)
5050
with warnings.catch_warnings(record=True) as w:
5151
warnings.simplefilter('always')
52-
out = bp.inputs.spike_current([(0, 50), (1, 50)])
52+
out = bp.inputs.spike_current(**kwargs)
5353
self.assertTrue(any(issubclass(x.category, DeprecationWarning) for x in w))
5454
self.assertIsNotNone(out)
55+
self.assertTrue(np.array_equal(np.asarray(out), np.asarray(bp.inputs.spike_input(**kwargs))))
5556

5657
def test_ramp_current_warns(self):
57-
# NOTE: like ``spike_current``, ``ramp_current`` also delegates to
58-
# ``constant_input`` rather than ``ramp_input`` (same wrapper defect),
59-
# so it expects ``[(value, duration), ...]`` pairs.
58+
# P16-C1: ``ramp_current`` now correctly delegates to ``ramp_input``
59+
# (it previously delegated to ``constant_input``). It must warn AND
60+
# accept ramp-style arguments.
6061
with warnings.catch_warnings(record=True) as w:
6162
warnings.simplefilter('always')
62-
out = bp.inputs.ramp_current([(0, 50), (1, 50)])
63+
out = bp.inputs.ramp_current(0., 1., 100.)
6364
self.assertTrue(any(issubclass(x.category, DeprecationWarning) for x in w))
6465
self.assertIsNotNone(out)
66+
self.assertTrue(np.array_equal(np.asarray(out), np.asarray(bp.inputs.ramp_input(0., 1., 100.))))
6567

6668

6769
@unittest.skipUnless(HAS_BRAINUNIT, 'brainunit required for unit-aware inputs')

brainpy/inputs/currents_test.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,3 +105,24 @@ def test_general2(self):
105105
bp.math.random.random((3, 10))],
106106
durations=[100, 300, 100])
107107
self.assertTrue(current.shape == (5000, 3, 10))
108+
109+
def test_spike_current_alias(self):
110+
# P16-C1: the deprecated ``spike_current`` alias must forward to
111+
# ``spike_input`` (it used to forward to ``constant_input`` and crash).
112+
import warnings
113+
kwargs = dict(sp_times=[10, 20, 30], sp_lens=1., sp_sizes=0.5, duration=40.)
114+
with warnings.catch_warnings():
115+
warnings.simplefilter('ignore', DeprecationWarning)
116+
aliased = bp.inputs.spike_current(**kwargs)
117+
direct = bp.inputs.spike_input(**kwargs)
118+
self.assertTrue(np.array_equal(np.asarray(aliased), np.asarray(direct)))
119+
120+
def test_ramp_current_alias(self):
121+
# P16-C1: the deprecated ``ramp_current`` alias must forward to
122+
# ``ramp_input`` (it used to forward to ``constant_input`` and crash).
123+
import warnings
124+
with warnings.catch_warnings():
125+
warnings.simplefilter('ignore', DeprecationWarning)
126+
aliased = bp.inputs.ramp_current(0., 1., 100.)
127+
direct = bp.inputs.ramp_input(0., 1., 100.)
128+
self.assertTrue(np.array_equal(np.asarray(aliased), np.asarray(direct)))

0 commit comments

Comments
 (0)