Skip to content

Fix BatchNorm/LieBN parameter resets and bias being applied when disabled - #44

Merged
bruAristimunha merged 5 commits into
spdlearn:mainfrom
lucas-heck:fix/batchnorm-init
Oct 19, 2026
Merged

bruAristimunha merged 5 commits into
spdlearn:mainfrom
lucas-heck:fix/batchnorm-init

Conversation

@lucas-heck

Copy link
Copy Markdown
Contributor

Problem

SPDBatchNormMean's was still applying an trainable bias when rebias=False.

After parametrization, reset_parameters() was modifying the temporary tensors instead of the underlying parameters, which was preventing biases and scales from resetting correctly in SPDBatchNormMean, SPDBatchNormMeanVar, and SPDBatchNormLie.

Fix

Only create and parametrize the layer’s bias when rebias is enabled.

Reset of parametrized parameters now is done through assignment, which invokes their right_inverse.

Verification

Added tests for disabled bias and LieBN resets after training across all the metrics.

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 81.25000% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
spd_learn/modules/batchnorm.py 64.70% 3 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

@bruAristimunha bruAristimunha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @lucas-heck, good catch. With rebias=False, the old code still applied the bias. That bias was created with torch.empty and never reset, so it was uninitialized memory, which makes this a real correctness fix.

Checked before merging:

  • Full suite with all four open PRs merged together: green.
  • SPDBatchNormMean / SPDBatchNormMeanVar: after perturbing every parameter, reset_parameters() restores the identity bias and unit weight, and the parameter objects stay the same (so optimizer state remains valid). LieBN is covered by your new test.

Minor, non-blocking:

  • Old SPDBatchNormMean(rebias=False) checkpoints contain parametrizations.bias.original, so load_state_dict(strict=True) will now report an unexpected key. Those weights were uninitialized anyway, so I'd just mention it in the release notes.
  • The "eye → assign if parametrized, else copy_" block now appears three times. A small shared helper could remove the repetition at some point, but it isn't needed for this PR.

I'll merge main into the branch to resolve the CHANGELOG.md conflict, then merge. Thanks!

@bruAristimunha
bruAristimunha merged commit 875ec45 into spdlearn:main Oct 19, 2026
10 checks passed
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.

3 participants