Fix MSTDP reset crashing when called before the first run - #795
Merged
Conversation
Follow-up to #794. MSTDP creates p_plus and p_minus lazily on the first update, because only then are the batch size and device known. #794 made reset_state_variables zero them unconditionally, so calling network.reset_state_variables() before the first run raised AttributeError: 'MSTDP' object has no attribute 'p_plus'. Building a network and resetting it before the first episode is a normal thing to do, so this was reachable. Declare p_plus and p_minus as None in __init__ alongside the other lazily built state, switch the update path's hasattr guards to 'is None' to match, and have the reset skip whatever has not been built. MSTDPET was never affected: it builds both in __init__. New test parametrised over MSTDP, MSTDPET and PostPre resets a freshly built network before running it. It fails for MSTDP without this change. Full suite 94 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Hananel-Hazan
added a commit
that referenced
this pull request
Sep 7, 2026
Brings in the six commits master gained today: four dependency bumps, the removal of the dead AbstractFeature.degrade hook (#793), the learning-rule reset fix (#794), and its follow-up for MSTDP's lazily built state (#795). No textual conflicts. The two files both sides touched, MCC_learning.py and topology_features.py, merged cleanly and the result is correct in both directions: this branch's perf work (the MSTDP fast path, the fold cache, the cached decay tensors) is intact, and master's reset fix reaches the learning rules through the feature chain as intended. Full suite on the merge: 182 passed, which includes this branch's test_perf_equivalence.py and test_learning_rule_specs.py alongside master's new reset tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Hananel-Hazan
added a commit
that referenced
this pull request
Sep 7, 2026
Master re-merged after #795 landed there as a squashed commit (7d2a66e). This branch already carried the same fix as the unsquashed f2d61ce, so git had two identities for one change and flagged the one place their text differs. Single conflict, in the parametrise list of test_reset_before_first_run_does_not_raise. Kept this branch's version, covering five rules including Hebbian and DiehlAndCook; master's covered three. DiehlAndCook exists only on this branch and the wider list is a superset, so it is the correct side. The resolved tree is identical to this branch's previous HEAD, so this joins the two histories without changing content. Recording it stops the same conflict recurring on every future merge from master. Full suite 186 passed, including all ten parametrised reset cases. black --check clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #794, which introduced this. My fault, caught while merging master into another branch.
The bug
MSTDPbuildsp_plusandp_minuslazily inside_connection_update, because only the first update knows the batch size and device. #794 madereset_state_variableszero them unconditionally, so this raised:...on any
network.reset_state_variables()called before the firstnetwork.run(). Building a network and resetting it before the first episode is a normal thing to do, so this was reachable, not theoretical.MSTDPETwas never affected: it builds both in__init__.PostPrewas never affected either.The fix
Declare
p_plusandp_minusasNoneinMSTDP.__init__alongside the other lazily-built state (_prev_source_s,_prev_target_s,eligibility), switch the update path'shasattrguards tois Noneso both spellings agree, and have the reset skip whatever has not been built yet.Test
New case parametrised over MSTDP, MSTDPET and PostPre: build a network, reset it before running, assert no exception. It fails for MSTDP without the source change and passes with it.
Full suite: 94 passed.
black --checkclean.🤖 Generated with Claude Code