Repository navigation
Clamp a given w to finite bounds in Conv1d/2d/3d and MeanField connections - #839
Merged
Merged
Conversation
…tions The given-w branch of these constructors clamped only when a bound was infinite (== instead of !=), so with finite bounds a given w was never clamped. Connection already used !=. Results change for code passing out-of-range weights with finite bounds; no built-in model or example does, and the golden tests are unchanged. Test: test_given_weights_are_clamped_to_finite_bounds fails on master for all four classes and passes with the fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Hananel-Hazan
added
the
tests-changed-ok
Both branches added tests at the end of test/network/test_connections.py; both sets are kept. Co-Authored-By: Claude Opus 5.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
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.
What this changes and why
Conv1dConnection,Conv2dConnection,Conv3dConnectionandMeanFieldConnectionnever clamped awpassed to the constructor when both bounds were finite. The given-
wbranch tested(self.wmin == -inf).any() or (self.wmax == inf).any(), the reverse ofConnection(!=): it clampedonly when a bound was infinite, where clamping does nothing. The four conditions now use
!=.The branches for
w=Nonewere right and are unchanged.Checklist
poetry run pytestlocally and it passed. (648 passed, on a machine with CUDA)masterand passes with my change(checked by the
bug-testcheck):test/network/test_connections.py::test_given_weights_are_clamped_to_finite_bounds(one case per class; all four fail on
master)- [x] Yes, only for code that passes these classes a
woutside finite bounds: its startingweights are now clamped. The golden files did not change and were not regenerated:
no golden network, built-in model or example passes such weights. The ANN-to-SNN
conversion passes
wwith no bounds and behaves as before. The "Results changes"entry is in
CHANGELOG.md.Unreleasedsection ofCHANGELOG.md.AI assistance
Tools used and for which parts: Claude Code found the reversed condition while testing the GPU-bounds
fix, wrote the fix, the test and the changelog entry.
Why the results changed
Before,
Conv2dConnection(..., w=w, wmin=0.0, wmax=1.0)kept every value ofw, including valuesoutside
[0, 1]. A learning rule clamped them at its first update (LearningRule.update); without alearning rule (
NoOp) they stayed out of range for the whole run. Now they are clamped atconstruction, as
Connectiondoes.