Skip to content

Fix Batch Norm - #423

Open
andrewdalpino wants to merge 2 commits into
masterfrom
fix-batch-norm
Open

Fix Batch Norm#423
andrewdalpino wants to merge 2 commits into
masterfrom
fix-batch-norm

Conversation

@andrewdalpino

Copy link
Copy Markdown
Member

BatchNorm normalizes by width, not batch size — src/NeuralNet/Layers/BatchNorm.php:184, 290-293. Matrix::variance() divides by row count (features); backward uses dOut->m() where batch size n() belongs. Masked by square test matrices.

@andrewdalpino
andrewdalpino requested review from a team and a lite review from Copilot August 16, 2026 02:10
@andrewdalpino andrewdalpino added the bug Something isn't working label Aug 16, 2026

Copilot AI 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.

Pull request overview

This PR fixes BatchNorm to normalize using the batch size (matrix n()) rather than the feature width (matrix m()), addressing incorrect statistics/gradients that were previously hidden by square (m == n) test matrices.

Changes:

  • Update forward-pass variance calculation to use a mean-of-squared-deviations formulation aligned to the batch dimension.
  • Fix backward gradient scaling to use dOut->n() (batch size) instead of dOut->m() (width/features).
  • Add a regression test using a non-square input matrix to ensure the bug can’t be masked.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/NeuralNet/Layers/BatchNormTest.php Adds a non-square (3×4) regression test to validate batch-dimension normalization in forward/backward/infer.
src/NeuralNet/Layers/BatchNorm.php Corrects variance computation and gradient scaling to normalize over batch size (n()).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@apphp

apphp commented Aug 16, 2026

Copy link
Copy Markdown

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants