Skip to content

Fix all known bugs for 0.3.5 (adopts #813 and #815) - #831

Merged
Hananel-Hazan merged 13 commits into
masterfrom
known-bugs-0.3.5
Oct 29, 2026
Merged

Hananel-Hazan merged 13 commits into
masterfrom
known-bugs-0.3.5

Conversation

@Hananel-Hazan

@Hananel-Hazan Hananel-Hazan commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

What this changes and why

This fixes every bug the test-hardening work (#828) pinned on master: the 25 entries of KNOWN_BUGS and the 15 strict expected failures in test/regression/test_invariants.py (40 in total). It also fixes the bugs found while doing that. On Hananel's decision, all of these fixes ship in 0.3.5, including the ones that change results. CHANGELOG.md lists those first, under Results changes.

Two contributor PRs are adopted, with the contributor kept as git author: #813 (MeanFieldConnection.normalize) and #815 (CSRM connection spike window). Both were reviewed with the bindsnet-pr-review skill. Both said no human had reviewed the code, so Hananel adopted them as accountable author. Two lines of #813's tests expected the batch-wide mean (0.75), which is the batch-mixing bug fixed here. They were corrected to the per-sample means.

Results changes (also in CHANGELOG)

  • Reset now clears reward-rule state. reset_state_variables() clears the MSTDP, MSTDPET and Rmax traces. Affects breakout_stdp.py and dot_tracing.py.
  • Reset now clears CSRM windows. CSRMNodes and its connection clear their spike windows on reset.
  • Sparse DiehlAndCook2015 now learns. DiehlAndCook2015(sparse=True) never changed its weights before. With one sample it now learns bit-identical weights to the dense model.
  • MeanFieldConnection applies weight_decay. It was ignored before.
  • No mixing across the batch. MeanFieldConnection and the MeanField feature use each sample's own mean. Batch size 1 is unchanged.

Saved outputs regenerated, each with its reason: csrm_window, mstdp_mstdpet, mstdp_mstdpet_dt05, rules_mix, rules_mix_dt05, meanfield_norm, meanfield_decay. In every case trial 0 is bit-identical where the change only acts after a reset. dc2015_b1 and dc2015_b16 only gained new state keys (the weights are now in state_dict); no value changed.

Evidence

  • Full suite locally: 601 passed, 0 skipped, 0 expected failures. Before: 513 passed, 40 expected failures.
  • Every fix has a test that fails on the old code. Each of these tests was checked against a deliberate wrong change named in its docstring.
  • Per-step code changes are pinned to the old formulas (test/network/test_perf_equivalence.py, CLAUDE.md rule):
    • MCC_learning.MSTDPET at batch size 1 is bit-identical to the old one-dimensional formula, at dt 1 and 0.5.
    • Batch 2 with sum equals the sum of single samples, to summation order (rtol 1e-6). The same holds for Rmax.
  • Operation counts at batch size 1: MCC_learning.MSTDPET 24 → 27, Rmax 24 → 26. The increase comes from per-sample state (batched matrix products and a reduction over the batch); regenerated with that reason.
  • Interface: the API snapshot adds 6 methods, and MeanField gains an optional name.
  • Not done: the sandboxed canary comparison (review.py --local-ref). It stopped at its gate because this branch changes binary golden files and .github/; it needs Hananel's clearance for the exact commit.

Checklist

  • poetry run pytest passed locally.
  • Every fix has a test that fails on master and passes here (bug-test check).
  • Results change: yes, listed above and in CHANGELOG; goldens regenerated with --reason.
  • Changed files: goldens, op_counts.json, api_surface.json, FIXED_BUGS (was KNOWN_BUGS). The weakened-tests check lists them: the removed test_known_bug_still_fails and the removed expected-failure markers are fixes, not weakening. Needs the tests-changed-ok label.
  • CHANGELOG Unreleased updated.
  • AI tool: Claude Code (Claude Opus 5.5). The adopted commits were written with OpenAI Codex.

🤖 Generated with Claude Code

Hananel-Hazan and others added 13 commits October 28, 2026 22:19
…SRMNodes reset clears its spike window

IzhikevichNodes sized its parameters by the n argument (None when only shape is
given) and never reshaped them; CSRMNodes contracted with 'i,kij->kj'. Both now
work for any shape; one-dimensional results are bit-identical (goldens
conv2d_if_izh and csrm_window trial 0 unchanged). CSRMNodes.reset_state_variables
now zeroes last_spikes, which changes results after a reset (csrm_window
golden regenerated with that reason).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…with one channel

LocalConnection.compute dropped the batch dimension in its final view.
MaxPool{1,2,3}d multiplied a missing decay (None) and squeeze()d away a channel
of size 1; a missing decay now means plain spike counts, and the firing-rate
buffer follows the spike tensor's shape. Results that ran before are unchanged
(goldens pass). Reproducers kept as test_fixed_bug_stays_fixed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…; NetworkMonitor reset; tuple target shapes

- MCC_LearningRule bounded sparse weights into a new tensor that only the rule
  held, so DiehlAndCook2015(sparse=True) never learned; the bound and the
  sparse normalisation now write in place. At batch 1 sparse and dense learn
  bit-identical weights without norm (test_sparse_dc2015_learns_like_dense);
  with norm they differ by summation order (max 6.0e-8).
- MulticompartmentConnection.compute applies per-sample sparse weight copies
  ([batch, src, tgt]) with bmm; one copy keeps the old arithmetic.
- NetworkMonitor.reset_state_variables looked connections up in layers.
- [batch] + target.shape failed for tuple shapes.

Results change: DiehlAndCook2015(sparse=True) now learns.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…work with batch > 1

- LearningRule.reset_state_variables deletes the tensors a rule creates on its
  first update (_step_state); AbstractConnection.reset_state_variables forwards
  to the rule. Reset then equals a freshly built network (invariant tests for
  mstdp_mstdpet and rules_mix now pass). Results change after a reset with
  learning on: goldens mstdp_mstdpet(_dt05), rules_mix(_dt05) regenerated,
  trial 0 bit-identical.
- Rmax and MCC_learning.MSTDPET keep per-sample state and reduce the update
  over the batch, as MSTDP does. Batch 1 is bit-identical to the old formula
  (TestMccMstdpetBatched, goldens); batch 2 with sum equals the sum of single
  samples to summation order. Op counts at batch 1: MCC MSTDPET 24 -> 27,
  Rmax 24 -> 26 (regenerated with reason). API snapshot: +1 method.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
)

Adopted from #813 (head 53dd2e3) by AHMETHAKANBEZIR1, reviewed with the
bindsnet-pr-review skill (sandboxed canaries, fail-then-pass, API, timing);
Hananel adopted it as accountable author because the PR says no human
reviewed it. MeanFieldConnection.normalize scales w in place by norm over the
absolute weight sum (a shared scalar counted once per target), instead of
assigning a plain tensor to the Parameter (TypeError on every call).

Co-Authored-By: Hananel Hazan <hananel@hazan.org.il>
Co-Authored-By: OpenAI Codex <codex@openai.com>
Adopted from #815 (head 49c57f8) by AHMETHAKANBEZIR1, reviewed with the
bindsnet-pr-review skill; Hananel adopted it as accountable author because the
PR says no human reviewed it. Connection.compute_window re-creates s_w when the
batch size changes, and reset_state_variables clears it for CSRM targets.

Co-Authored-By: Hananel Hazan <hananel@hazan.org.il>
Co-Authored-By: OpenAI Codex <codex@openai.com>
meanfield_norm and csrm_window goldens regenerated (both used to record a
crash). Invariant xfails removed: reset-equals-fresh for csrm_window and
meanfield_norm, round trip and batch for meanfield_norm. Still open: the CSRM
spike window is not in the state_dict (round trip).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ction drives CSRMNodes

- Connection.s_w is a registered buffer, so state_dict saves it; a load adopts
  a saved window or drops one the saved state lacked (_load_window). The
  csrm_window round-trip invariant now passes; golden regenerated (adds s_w,
  no value changed).
- MulticompartmentConnection.compute_window read s_w and self.w, neither of
  which it has, so MCC into CSRMNodes always raised. It now keeps a window of
  its own outputs (pipeline runs once per step), resized on a batch change,
  cleared on reset, saved in the state_dict. With static weights it matches a
  Connection bit for bit (test_multicompartment_into_csrm_matches_connection).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…_decay

- compute() took the mean over the whole batch, so one sample's input depended
  on the others; it now uses each sample's own mean (batch 1 bit-identical:
  goldens pass without the weight_decay change).
- weight_decay was passed by position into AbstractConnection's reduction
  slot and ignored; it is now passed by name. Results change for anyone who
  set weight_decay on a MeanFieldConnection (golden meanfield_decay
  regenerated with that reason).
- Two #813 test lines expected the batch-wide mean (0.75); corrected to the
  per-sample means.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…on features at batch > 1, sparse weight bounds

- topology_features.MeanField never called the base constructor (no name) and
  had no initial value; it now has an optional name, a placeholder value, and
  a per-sample mean ([batch, src, tgt], applied per sample by compute).
- Sub-features (Normalization, Updating) get no-op prime_feature, update,
  normalize and reset_state_variables, which MulticompartmentConnection calls
  on every pipeline entry; both now work in the constructor and in Network.run.
- AdaptationBaseSynapsHistory/OtherSynaps: a synapse is active when its
  pre-synaptic neuron spiked in any sample (the mask is shared connectivity);
  batch 1 gives the same values.
- Sparse weights with wmin/wmax: Connection.__init__ bounds a dense copy,
  LearningRule.update bounds in place via a dense copy, and
  WeightDependentPostPre/DiehlAndCook compute their factors from a dense view.
  PostPre, WeightDependentPostPre and DiehlAndCook on a bounded
  SparseConnection learn bit-identical weights to the dense Connection.
- Test workarounds for these bugs removed (batch-1-only sets, broken pairs,
  tuple-shape and sparse-bound special cases).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…mpartmentConnection feature values in the state_dict

- MulticompartmentConnection saves its pipeline feature values as
  feature.<name> and loads them in place, so DiehlAndCook2015 can be saved
  and restored (the last four strict xfails in test_invariants.py now pass;
  dc2015 goldens regenerated: new keys, no value changed).
- Every KNOWN_BUGS entry is fixed: the dict becomes FIXED_BUGS (reproducers
  that must keep running); the empty xfail machinery in test_invariants.py and
  the batch-1, no-learning-check and sub-feature workarounds in
  test_all_classes.py are removed, so those classes run at batch 2 with full
  checks.
- CONTRIBUTING, PR template and bindsnet/learning/README.md updated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Network (reviewer B):
- MulticompartmentConnection state_dict: refuses to save duplicate feature
  names (they shared one key, and loading put one feature's value into the
  other); checks shape and sparse layout on load (copy_ broadcast a wrong-shaped
  tensor silently); saves the adaptation features' mask, spike buffer and
  counter (only value was saved).
- An Updating sub-feature drives its parent's learning; the connection's
  update skips that parent (it learned twice per step).
- A sub-feature whose parent is not in the pipeline is refused at construction
  (it failed later with an unclear TypeError). insert_pipeline had list.insert's
  arguments swapped.
- Connection.compute_window and MaxPool resets create state on the spikes'
  device (CUDA device mismatch, also on master).
- Docstrings state how the two CSRM windows differ once weights learn.

Learning rules (reviewer A):
- MCC_learning.DiehlAndCook crashed on sparse weights (twin of the learning.py
  fix). Sparse MCC weight copies now all get the batch's shared update in
  PostPre, DiehlAndCook and MSTDPET (as MSTDP did), so sparse equals dense; the
  default squeeze reduction means a sum over a larger batch, as in the fused
  dense path (_reduce_batch, _add_to_feature).
- Docs: per-sample reward shape [batch, 1, 1]; reset keeps the reduction chosen
  at construction.

CHANGELOG: checkpoint compatibility (strict=False for old checkpoints), new
state shapes, sparse and Updating semantics. Each fix has a test that fails on
ce98f93 and passes here.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Hananel-Hazan Hananel-Hazan added the tests-changed-ok Maintainer reviewed every item the weakened-tests check listed label Oct 29, 2026
@Hananel-Hazan
Hananel-Hazan merged commit d48dc79 into master Oct 29, 2026
12 of 13 checks passed
Hananel-Hazan added a commit that referenced this pull request Oct 29, 2026
master's tree equals 02219bf, the #831 head this branch was built on, so the
branch's files are kept unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Hananel-Hazan added a commit that referenced this pull request Oct 29, 2026
* fix: IzhikevichNodes and CSRMNodes accept multi-dimensional shapes; CSRMNodes reset clears its spike window

IzhikevichNodes sized its parameters by the n argument (None when only shape is
given) and never reshaped them; CSRMNodes contracted with 'i,kij->kj'. Both now
work for any shape; one-dimensional results are bit-identical (goldens
conv2d_if_izh and csrm_window trial 0 unchanged). CSRMNodes.reset_state_variables
now zeroes last_spikes, which changes results after a reset (csrm_window
golden regenerated with that reason).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: LocalConnection batch > 1; MaxPool connections without decay or with one channel

LocalConnection.compute dropped the batch dimension in its final view.
MaxPool{1,2,3}d multiplied a missing decay (None) and squeeze()d away a channel
of size 1; a missing decay now means plain spike counts, and the firing-rate
buffer follows the spike tensor's shape. Results that ran before are unchanged
(goldens pass). Reproducers kept as test_fixed_bug_stays_fixed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: sparse MulticompartmentConnection learns and runs with batch > 1; NetworkMonitor reset; tuple target shapes

- MCC_LearningRule bounded sparse weights into a new tensor that only the rule
  held, so DiehlAndCook2015(sparse=True) never learned; the bound and the
  sparse normalisation now write in place. At batch 1 sparse and dense learn
  bit-identical weights without norm (test_sparse_dc2015_learns_like_dense);
  with norm they differ by summation order (max 6.0e-8).
- MulticompartmentConnection.compute applies per-sample sparse weight copies
  ([batch, src, tgt]) with bmm; one copy keeps the old arithmetic.
- NetworkMonitor.reset_state_variables looked connections up in layers.
- [batch] + target.shape failed for tuple shapes.

Results change: DiehlAndCook2015(sparse=True) now learns.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: MSTDP/MSTDPET/Rmax state cleared on reset; Rmax and MCC MSTDPET work with batch > 1

- LearningRule.reset_state_variables deletes the tensors a rule creates on its
  first update (_step_state); AbstractConnection.reset_state_variables forwards
  to the rule. Reset then equals a freshly built network (invariant tests for
  mstdp_mstdpet and rules_mix now pass). Results change after a reset with
  learning on: goldens mstdp_mstdpet(_dt05), rules_mix(_dt05) regenerated,
  trial 0 bit-identical.
- Rmax and MCC_learning.MSTDPET keep per-sample state and reduce the update
  over the batch, as MSTDP does. Batch 1 is bit-identical to the old formula
  (TestMccMstdpetBatched, goldens); batch 2 with sum equals the sum of single
  samples to summation order. Op counts at batch 1: MCC MSTDPET 24 -> 27,
  Rmax 24 -> 26 (regenerated with reason). API snapshot: +1 method.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: preserve the mean-field weight parameter during normalization (#813)

Adopted from #813 (head 53dd2e3) by AHMETHAKANBEZIR1, reviewed with the
bindsnet-pr-review skill (sandboxed canaries, fail-then-pass, API, timing);
Hananel adopted it as accountable author because the PR says no human
reviewed it. MeanFieldConnection.normalize scales w in place by norm over the
absolute weight sum (a shared scalar counted once per target), instead of
assigning a plain tensor to the Parameter (TypeError on every call).

Co-Authored-By: Hananel Hazan <hananel@hazan.org.il>
Co-Authored-By: OpenAI Codex <codex@openai.com>

* fix: reset and resize the CSRM connection spike history (#815)

Adopted from #815 (head 49c57f8) by AHMETHAKANBEZIR1, reviewed with the
bindsnet-pr-review skill; Hananel adopted it as accountable author because the
PR says no human reviewed it. Connection.compute_window re-creates s_w when the
batch size changes, and reset_state_variables clears it for CSRM targets.

Co-Authored-By: Hananel Hazan <hananel@hazan.org.il>
Co-Authored-By: OpenAI Codex <codex@openai.com>

* test: goldens and invariants after adopting #813 and #815

meanfield_norm and csrm_window goldens regenerated (both used to record a
crash). Invariant xfails removed: reset-equals-fresh for csrm_window and
meanfield_norm, round trip and batch for meanfield_norm. Still open: the CSRM
spike window is not in the state_dict (round trip).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: CSRM spike window saved in the state_dict; MulticompartmentConnection drives CSRMNodes

- Connection.s_w is a registered buffer, so state_dict saves it; a load adopts
  a saved window or drops one the saved state lacked (_load_window). The
  csrm_window round-trip invariant now passes; golden regenerated (adds s_w,
  no value changed).
- MulticompartmentConnection.compute_window read s_w and self.w, neither of
  which it has, so MCC into CSRMNodes always raised. It now keeps a window of
  its own outputs (pipeline runs once per step), resized on a batch change,
  cleared on reset, saved in the state_dict. With static weights it matches a
  Connection bit for bit (test_multicompartment_into_csrm_matches_connection).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: MeanFieldConnection keeps batch samples apart and applies weight_decay

- compute() took the mean over the whole batch, so one sample's input depended
  on the others; it now uses each sample's own mean (batch 1 bit-identical:
  goldens pass without the weight_decay change).
- weight_decay was passed by position into AbstractConnection's reduction
  slot and ignored; it is now passed by name. Results change for anyone who
  set weight_decay on a MeanFieldConnection (golden meanfield_decay
  regenerated with that reason).
- Two #813 test lines expected the batch-wide mean (0.75); corrected to the
  per-sample means.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: MeanField feature, Normalization/Updating sub-features, adaptation features at batch > 1, sparse weight bounds

- topology_features.MeanField never called the base constructor (no name) and
  had no initial value; it now has an optional name, a placeholder value, and
  a per-sample mean ([batch, src, tgt], applied per sample by compute).
- Sub-features (Normalization, Updating) get no-op prime_feature, update,
  normalize and reset_state_variables, which MulticompartmentConnection calls
  on every pipeline entry; both now work in the constructor and in Network.run.
- AdaptationBaseSynapsHistory/OtherSynaps: a synapse is active when its
  pre-synaptic neuron spiked in any sample (the mask is shared connectivity);
  batch 1 gives the same values.
- Sparse weights with wmin/wmax: Connection.__init__ bounds a dense copy,
  LearningRule.update bounds in place via a dense copy, and
  WeightDependentPostPre/DiehlAndCook compute their factors from a dense view.
  PostPre, WeightDependentPostPre and DiehlAndCook on a bounded
  SparseConnection learn bit-identical weights to the dense Connection.
- Test workarounds for these bugs removed (batch-1-only sets, broken pairs,
  tuple-shape and sparse-bound special cases).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test: known-bug list becomes FIXED_BUGS; workarounds removed; MulticompartmentConnection feature values in the state_dict

- MulticompartmentConnection saves its pipeline feature values as
  feature.<name> and loads them in place, so DiehlAndCook2015 can be saved
  and restored (the last four strict xfails in test_invariants.py now pass;
  dc2015 goldens regenerated: new keys, no value changed).
- Every KNOWN_BUGS entry is fixed: the dict becomes FIXED_BUGS (reproducers
  that must keep running); the empty xfail machinery in test_invariants.py and
  the batch-1, no-learning-check and sub-feature workarounds in
  test_all_classes.py are removed, so those classes run at batch 2 with full
  checks.
- CONTRIBUTING, PR template and bindsnet/learning/README.md updated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs: CHANGELOG for the 0.3.5 bug fixes, results changes listed first

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: issues found by independent review of PR #831

Network (reviewer B):
- MulticompartmentConnection state_dict: refuses to save duplicate feature
  names (they shared one key, and loading put one feature's value into the
  other); checks shape and sparse layout on load (copy_ broadcast a wrong-shaped
  tensor silently); saves the adaptation features' mask, spike buffer and
  counter (only value was saved).
- An Updating sub-feature drives its parent's learning; the connection's
  update skips that parent (it learned twice per step).
- A sub-feature whose parent is not in the pipeline is refused at construction
  (it failed later with an unclear TypeError). insert_pipeline had list.insert's
  arguments swapped.
- Connection.compute_window and MaxPool resets create state on the spikes'
  device (CUDA device mismatch, also on master).
- Docstrings state how the two CSRM windows differ once weights learn.

Learning rules (reviewer A):
- MCC_learning.DiehlAndCook crashed on sparse weights (twin of the learning.py
  fix). Sparse MCC weight copies now all get the batch's shared update in
  PostPre, DiehlAndCook and MSTDPET (as MSTDP did), so sparse equals dense; the
  default squeeze reduction means a sum over a larger batch, as in the fused
  dense path (_reduce_batch, _add_to_feature).
- Docs: per-sample reward shape [batch, 1, 1]; reset keeps the reduction chosen
  at construction.

CHANGELOG: checkpoint compatibility (strict=False for old checkpoints), new
state shapes, sparse and Updating semantics. Each fix has a test that fails on
ce98f93 and passes here.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* ci: close test-guard bypasses found by independent review (WIP part 1)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test: op-count workloads for every Nodes class

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* ci: close the test-guard bypasses found by independent review (part 2), docs

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: AHMETHAKANBEZIR1 <ahmethakan0008@gmail.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests-changed-ok Maintainer reviewed every item the weakened-tests check listed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants