Skip to content

fix(loss): use autograd-aware all_reduce in BarlowTwinsLoss#1980

Open
saud5150 wants to merge 3 commits into
lightly-ai:masterfrom
saud5150:fix/barlow-twins-gather-distributed-gradient
Open

fix(loss): use autograd-aware all_reduce in BarlowTwinsLoss#1980
saud5150 wants to merge 3 commits into
lightly-ai:masterfrom
saud5150:fix/barlow-twins-gather-distributed-gradient

Conversation

@saud5150

Copy link
Copy Markdown
Contributor

closes #1977

Description

  • My change is breaking

BarlowTwinsLoss(gather_distributed=True) reduced the cross-correlation matrix
with the raw torch.distributed.all_reduce, which isn't autograd-aware. The
forward value was correct, but the backward pass scaled the gradient down by
1/world_size under DDP — silently. Swapped it for
torch.distributed.nn.all_reduce, whose backward is itself an all_reduce, so the
gradient is correct and the forward value is unchanged. Same fix #1923 landed for
SIGReg; Barlow Twins was the last loss with the pattern.

Tests

  • My change is covered by existing tests.
  • My change needs new tests.
  • I have added/adapted the tests accordingly.
  • I have manually tested the change.

Added two tests (world_size=2, two identical ranks): forward matches the
non-distributed loss on the global batch, and the gradient matches single-GPU
Barlow Twins on the global batch. The gradient test fails on master and passes
with the fix. make static-checks and the loss suite pass.

Documentation

  • I have added docstrings to all public functions/methods.
  • My change requires a change to the documentation ( .rst files).
  • I have updated the documentation accordingly.
  • The autodocs update the documentation accordingly.

Internal implementation only — no API, docstring, or .rst changes.

Implications / comments / further issues

None. Public API, forward loss value, and single-GPU behavior unchanged; only the
multi-GPU gradient is corrected.

The raw torch.distributed.all_reduce overwrote the cross-correlation
matrix in place and was not registered in the autograd graph. Under DDP
gradient averaging this scaled the backbone gradient down by
1/world_size relative to single-GPU Barlow Twins on the global batch.

Swap it for the autograd-aware torch.distributed.nn.all_reduce, whose
backward is itself an all_reduce, so the gradient is scaled back up by
world_size. The forward value is unchanged.

Fixes lightly-ai#1977. Same fix as SIGReg in lightly-ai#1923.

@gabrielfruet gabrielfruet 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.

/review

Comment thread tests/loss/test_barlow_twins_loss.py Outdated
Comment thread lightly/loss/barlow_twins_loss.py
Mocking all_reduce only tested the mock, not the real collective, and
would break on any change to the reduction. Real multi-rank coverage
is tracked in lightly-ai#1982.
@saud5150

Copy link
Copy Markdown
Contributor Author

Kept the manual division — agreed, gloo not supporting ReduceOp.AVG makes it the safer choice for the CPU path. Removed the two mocked tests; real distributed coverage will come through #1982 (happy to add a BarlowTwins case once that context lands).

@liopeer liopeer requested a review from gabrielfruet July 15, 2026 07:27

@gabrielfruet gabrielfruet 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.

LGTM!! Thank you for the nice work

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.

[BUG] BarlowTwinsLoss with gather_distributed=True produces gradient which is off by 1/world_size under DDP

3 participants