Skip to content

Preserve the mean-field weight parameter during normalization - #813

Open
AHMETHAKANBEZIR1 wants to merge 3 commits into
BindsNET:masterfrom
AHMETHAKANBEZIR1:fix/meanfield-normalization-parameter
Open

AHMETHAKANBEZIR1 wants to merge 3 commits into
BindsNET:masterfrom
AHMETHAKANBEZIR1:fix/meanfield-normalization-parameter

Conversation

@AHMETHAKANBEZIR1

@AHMETHAKANBEZIR1 AHMETHAKANBEZIR1 commented Oct 21, 2026 •

Copy link
Copy Markdown

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:

  • The updated review cases fail 10 times on the preceding implementation, with 17 passing controls; all 27 cases pass after the revision.
  • Complete standalone suite: 282 passed, 1 skipped. Coverage includes negative and cancelling weights, shared/per-target compute equivalence, repeated normalization, zero and disabled norms, Parameter identity and Network.run.
  • Private integration with Reset and resize CSRM connection spike history #815 and Forward mean-field learning rule options by name #816: 297 passed, 1 skipped. The changes applied cleanly apart from retaining each changelog entry. This is compatibility evidence, not a requirement to merge the PRs together.
  • Configured isort/Black hooks and diff checks passed. The five current-head GitHub checks succeeded. CUDA and timing measurements were not run locally.

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.

@Hananel-Hazan Hananel-Hazan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. 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]. With norm=1, a shared weight of -0.5 becomes 1.0. An inhibitory connection silently becomes an excitatory one.
  2. 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.
  3. A shared weight and per-neuron weights give different results. For 3 target neurons and norm=2, a shared weight of 0.25 becomes 2.0, but per-neuron weights [0.25, 0.25, 0.25] become 0.67 each. 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 norm description 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.

AHMETHAKANBEZIR1 and others added 3 commits October 23, 2026 04:47
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
@AHMETHAKANBEZIR1

Copy link
Copy Markdown
Author

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.

This branch has not been deployed

No deployments
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.

2 participants