Repository navigation
Preserve the mean-field weight parameter during normalization - #813
AHMETHAKANBEZIR1 wants to merge 3 commits into
Conversation
Hananel-Hazan
left a comment
There was a problem hiding this comment.
Hi AhmetHakanBezir,
Thank you for using BindsNET and for opening this PR. The crash fix is right: on master, normalize() fails in every case I tried, and with your change the weight stays the same registered parameter. A couple of things need to change before it can merge. I measured each of them on your branch:
- Negative weights flip sign. The code divides by the plain sum of the weights, and the sum of negative weights is negative. With
norm=2, weights[-1, -2]become[0.67, 1.33]. Withnorm=1, a shared weight of-0.5becomes1.0. An inhibitory connection silently becomes an excitatory one. - Weights become infinite when their signs cancel.
[1, -1]becomes[inf, -inf], and[1, -0.999999]becomes about[986895, -986894]. The guard only handles weights that are all zero. - A shared weight and per-neuron weights give different results. For 3 target neurons and
norm=2, a shared weight of0.25becomes2.0, but per-neuron weights[0.25, 0.25, 0.25]become0.67each. That is the same network, with three times more input in one case.
Please divide by the sum of the absolute values, which is what Connection.normalize already does, and count a shared weight once for each target neuron:
if self.norm is not None:
# A scalar weight is shared by every target neuron.
total = self.w.abs().sum() * (self.target.n / self.w.numel())
self.w *= self.norm / total.masked_fill(total == 0, 1)I tried this on your branch:
- signs stay the same (
[-1, -2]becomes[-0.67, -1.33]); [1, -1]stays[1, -1];- the shared and per-neuron cases now agree at
0.67; - it runs on GPU with no extra synchronization, at the same speed as your version.
Your three shared-weight tests expect norm itself and will need the new expected values.
Also:
- Add tests for negative weights, for mixed-sign weights that sum to zero, and for a shared weight compared with an equal per-neuron vector.
- Update the
normdescription in__init__, which still says "Total weight per target neuron", so it matches the new docstring. - Rebase on master when it's ready; the changelog entry will need a small conflict fix.
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
4025c6a to
53dd2e3
Compare
|
Implemented in 53dd2e3: normalization uses the absolute weight total, counts a shared weight once per target neuron, preserves inhibitory signs, and keeps zero/cancelling weights finite. The constructor documentation and regression expectations are updated, and the branch includes master 78ec02b. All 27 mean-field tests pass; the full CPU suite has 282 passes and 1 skip. All five current-head CI checks are green. CUDA and timing were not measured locally. |
MeanFieldConnection.normalize replaces its registered Parameter with a reshaped tensor and crashes for shared and per-target weights. Scale the existing Parameter in place, preserving its identity, shape and state_dict registration.
Apply the normalization requested in review: norm is the total absolute weight across target neurons. Divide by the sum of absolute values to preserve inhibitory signs and prevent cancellation, and count a shared scalar once per target neuron. With three targets and norm=2, both a shared weight of 0.25 and the equivalent per-target vector normalize to 2/3 at every target. Zero weights remain finite. Constructor and method documentation now use this convention.
The branch includes master 78ec02b, including the merged #814 kernel-normalization change; the changelog conflict was resolved with all entries retained.
Validation on Windows CPU, Python 3.12 / Torch 2.14.0:
AI disclosure: OpenAI Codex autonomously prepared and tested this contribution, including the requested revision. The account owner has not independently reviewed the code. No personal production incident or task-accuracy improvement is claimed.