Skip to content

Remove dead AbstractFeature.degrade hook - #793

Merged
Hananel-Hazan merged 1 commit into
masterfrom
remove-dead-degrade-hook
Sep 7, 2026
Merged

Remove dead AbstractFeature.degrade hook#793
Hananel-Hazan merged 1 commit into
masterfrom
remove-dead-degrade-hook

Conversation

@Hananel-Hazan

Copy link
Copy Markdown
Collaborator

Supersedes #792. Thanks to @Anai-Guo for finding the bug.

What is wrong

AbstractFeature.degrade calls itself, passing an argument the method does not accept:

def degrade(self) -> None:
    return self.degrade(self.value)

degrade takes only self, so this raises TypeError on the very first call, for every feature. It is not even infinite recursion.

Why removing rather than fixing

git log -L 286,295:bindsnet/network/topology_features.py returns a single commit, 4b577e9 "Add topology_features.py". The self-call has been there since the file was created, so the method has never run successfully once.

Nothing calls it. Grepping the whole repo across all file types, the only occurrences of degrade as a method are the definition and the broken self-call inside it. No caller in the library, examples, tests, or docs.

It looks like an intended third per-feature lifecycle hook alongside update and normalize, which MulticompartmentConnection does call generically over its feature pipeline (for f in self.pipeline: f.update(**kwargs) and f.normalize() in topology.py). The matching for f in self.pipeline: f.degrade() loop was never written. Degradation ended up in Degradation.compute instead, which is where the pipeline applies every feature, and which already guards against degrade_function being None.

Fixing the typo would leave two spellings of the same operation, the fixed degrade() being the unguarded one. It would also leave a method on the abstract base that only Degradation can satisfy, since degrade_function is set solely by Degradation.__init__ — calling degrade() on a Weight or Mask would still fail, just with AttributeError.

Risk

None that I can find. Any downstream caller would be crashing today.

Verification

Full suite on this branch: 83 passed. black --check clean.

🤖 Generated with Claude Code

AbstractFeature.degrade called itself with an argument it does not
accept, so it raised TypeError on the first call for every feature. It
has been that way since 4b577e9 added topology_features.py, and nothing
in the repo has ever called it.

It looks like an intended third per-feature lifecycle hook alongside
update and normalize, which MulticompartmentConnection does call over
its pipeline. The matching 'for f in self.pipeline: f.degrade()' loop
was never written; degradation lives in Degradation.compute instead,
which is where the pipeline applies it and which already guards against
degrade_function being None.

Fixing the method would leave two spellings of the same operation, one
of them unguarded, so remove it. Reported by @Anai-Guo in #792.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Hananel-Hazan
Hananel-Hazan merged commit c07f3b5 into master Sep 7, 2026
10 checks passed
@Hananel-Hazan
Hananel-Hazan deleted the remove-dead-degrade-hook branch September 7, 2026 02:08
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant