Fix LeakyParallel silently ignoring a per-neuron beta; correct the reset=zero formula in Leaky - #441
Merged
ixfd64 merged 1 commit intoSep 20, 2026
Conversation
…set="zero" formula in Leaky
### Problem
**1. `LeakyParallel` drops a per-neuron `beta`.**
`_beta_to_weight_hh` walks an `if / elif / elif / else` chain:
if isinstance(self.beta, (float, int)): ...
elif isinstance(self.beta, torch.Tensor) or isinstance(self.beta, torch.FloatTensor):
if len(self.beta) == 1: ...
elif len(self.beta) == self.hidden_size: # <-- sibling of the Tensor elif
for i in range(self.hidden_size): ...
else:
raise ValueError(...)
`_beta_buffer` always stores `beta` as a `torch.Tensor`, so control always
enters the second branch and only its inner `if len(self.beta) == 1` is
reachable. The `elif len(self.beta) == self.hidden_size` (the per-neuron
path) and the `else: raise ValueError` are dead. Passing
`beta=<tensor of length hidden_size>` -- which the docstring explicitly
supports ("multi-valued (one weight per neuron)") -- leaves
`rnn.weight_hh_l0` at its random RNN initialization, with no error. The
layer then trains and runs with arbitrary recurrent decay rates instead
of the requested ones.
**2. `Leaky` docstring for `reset_mechanism="zero"` is wrong.**
It states `U[t+1] = βU[t] + I_syn[t+1] - R(βU[t] + I_in[t+1])`, i.e.
`(1-R)(βU[t] + I)`, which is `0` on the step after a spike regardless of
input. The implementation (`_base_zero`) is standard reset-then-integrate,
`U[t+1] = β(1-R)U[t] + I_in[t+1]`, which equals `I_in` on that step. Also
`Leaky` has no synaptic current, so `I_syn` is a copy-paste from
`Synaptic`.
### Changes
- `snntorch/_neurons/leakyparallel.py`: nest the two length checks (and
the `ValueError`) inside the `torch.Tensor` branch so the per-neuron
path is reached; a genuinely unsupported `beta` type now raises
`TypeError` instead of falling through silently.
- `snntorch/_neurons/leaky.py`: fix the `reset_mechanism="zero"` formula
and drop the stray `I_syn`.
### Tests
New `tests/test_snntorch/test_leakyparallel.py` (5 tests):
- scalar `beta` still fills the diagonal,
- **per-neuron `beta` (len == hidden_size) is written to the diagonal**
(fails without this change),
- length-1 `beta` tensor still works,
- **a bad-length `beta` raises `ValueError`** (fails without this change --
it was silently accepted),
- **two layers identical but for a per-neuron `beta` produce different
outputs** (fails without this change -- both ran at the default
recurrent weights).
Full suite: 195 -> 200 passed, 2 xfailed.
Collaborator
|
I don't see any issues. Let me know once the PR is ready for review. |
tritsystem
marked this pull request as ready for review
September 4, 2026 07:13
Contributor
Author
|
Thanks for taking a look @ixfd64 — I've just moved it out of draft, it's ready for review now. Quick recap of what it fixes:
5 new tests in |
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
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.
Fixes #442.
Problem
1.
LeakyParallelsilently drops a per-neuronbeta._beta_to_weight_hhwalks anif / elif / elif / elsechain:_beta_bufferalways storesbetaas atorch.Tensor, so control alwaysenters the second branch, and only its inner
if len(self.beta) == 1isreachable. The
elif len(self.beta) == self.hidden_size(the per-neuron path)and the
else: raise ValueErrorare dead.Passing
beta=<tensor of length hidden_size>— which the docstring explicitlysupports ("multi-valued (one weight per neuron)") — leaves
rnn.weight_hh_l0at its random RNN initialisation, with no error. Thelayer then trains and runs with arbitrary recurrent decay rates. A
wrong-length
betais likewise accepted silently.Reproduce (before this PR):
2.
Leakydocstring —reset_mechanism="zero"formula is wrong.It states
U[t+1] = βU[t] + I_syn[t+1] - R(βU[t] + I_in[t+1]), i.e.(1-R)(βU[t] + I), which is0on the step after a spike regardless ofinput. The implementation (
_base_zero) is standard reset-then-integrate,U[t+1] = β(1-R)U[t] + I_in[t+1], which equalsI_inon that step (verifiedagainst a hand-rolled reference — matches to
0.0).Leakyalso has nosynaptic current, so
I_synis a copy-paste fromSynaptic.Changes
snntorch/_neurons/leakyparallel.py— nest the two length checks and theValueErrorinside thetorch.Tensorbranch so the per-neuron path isreached; an unsupported
betatype now raisesTypeErrorinstead offalling through.
snntorch/_neurons/leaky.py— fix thereset_mechanism="zero"formula anddrop the stray
I_syn.Tests
New
tests/test_snntorch/test_leakyparallel.py(5 tests). Three fail onmasterwithout this change:betafills the diagonalbeta(len == hidden_size) written to the diagonalbetatensor still worksbetaraisesValueErrorbetagive different outputFull suite:
195 → 200 passed, 2 xfailed.After this PR:
Checklist
flake8andblack(changed files clean;flake8 --select=E9,F63,F7,F82clean)