Skip to content

test: add shared gloo process pool for distributed loss tests#1983

Draft
saud5150 wants to merge 3 commits into
lightly-ai:masterfrom
saud5150:test/distributed-pytest-pool
Draft

test: add shared gloo process pool for distributed loss tests#1983
saud5150 wants to merge 3 commits into
lightly-ai:masterfrom
saud5150:test/distributed-pytest-pool

Conversation

@saud5150

Copy link
Copy Markdown
Contributor

Part of #1982 (PR 1 — draft for early feedback)

Stacked on #1980: the gradient regression test needs that fix (master still has
the raw dist.all_reduce), so #1980's two commits show in this diff until it
merges — I'll rebase onto master then. Review only the four files listed below.

Description

  • My change is breaking

Distributed loss behaviour is currently tested either in a separate module
(tests/utils/test_dist__gather__losses.py) disconnected from each loss's own
tests, or by mocking torch.distributed — which only exercises the mock, not the
real collective (exactly the gap that hid #1977 until the mock tests were reverted
in #1980).

This adds a torchmetrics-style session-scoped gloo (CPU) process pool, started
once per session and reused, so a test can submit real multi-rank work cheaply.
Following @liopeer's guidance in #1982:

  • tests/ddp_helpers.py (new): USE_PYTEST_POOL flag (default off, folds in the
    gloo/non-Windows checks) and setup_ddp. Adapted from and credited to
    torchmetrics' conftest.py.
  • tests/conftest.py: pytest_sessionstart/pytest_sessionfinish start/stop the
    pool; registers the DDP marker. Nothing is enabled in CI.
  • tests/loss/test_barlow_twins_loss.py: first consumer — two real 2-rank tests
    (@pytest.mark.DDP + skipif(not USE_PYTEST_POOL)). The gradient test is a
    regression test for [BUG] BarlowTwinsLoss with gather_distributed=True produces gradient which is off by 1/world_size under DDP #1977 and fails on the raw dist.all_reduce.
  • pyproject.toml: --import-mode=importlib in addopts. Required because the
    pool uses spawn, whose workers re-import the test module and deadlock under
    pytest's default prepend mode. It is a no-op for the rest of the suite.

Three things worth your review, since you asked to iterate: the submission API
(pytest.pool.starmap(worker, ...), workers return results to the parent to
assert); USE_PYTEST_POOL folding the backend check into the flag rather than a
separate skip condition; and the Barlow tests using identical data per rank
(catches the 1/world_size scaling bug but not heterogeneous-rank bugs).

Tests

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

Two new tests in tests/loss/test_barlow_twins_loss.py run on the pool; the rest
of the suite is unaffected (flag off → they skip). Verified on macOS/Apple Silicon,
Python 3.9, torch 2.8:

make static-checks                                   # format-check + type-check pass
pytest tests/loss/test_barlow_twins_loss.py          # flag off: 4 passed, 2 skipped
USE_PYTEST_POOL=1 pytest tests/loss/test_barlow_twins_loss.py   # 6 passed (pool runs)

Before/after on the regression test (USE_PYTEST_POOL=1, gradient test only):

raw dist.all_reduce (master):   GRAD MATCH: False   # gradient scaled by 1/world_size
dist_nn.all_reduce  (fixed):    GRAD MATCH: True     # forward is identical either way

Full make test-fast under the new import mode: 4 failed, 1687 passed — the 4 are
the known pre-existing macOS/Python-3.9 env failures (PyAV pickling, a py3.9
assertWarns bug, the existing gloo DCL mismatch), unchanged by this PR and not hit
by CI (Ubuntu 3.7/3.12).

Documentation

  • I have added docstrings to all changed/added 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.

Test-only change; no public API or .rst docs — no public functions added (the
pool module, hooks, and setup_ddp are internal; the worker helpers use inline
comments per the surrounding test file's style).

Implications / comments / further issues

  • Follow-ups within Proper distributed tests #1982 (deliberately out of scope here to keep PR 1 small):
    migrate the existing losses in test_dist__gather__losses.py onto the pool;
    add a heterogeneous-data-per-rank case; enable the pool in CI once it's proven
    green on Linux; revisit pytest.pool (global attr) vs a fixture.
  • Not yet confirmed under pytest on Linux CI — the spawn/importlib mechanism is
    platform-independent, but a maintainer sanity-check there would be valuable before
    any CI-enablement PR. Happy to iterate.

Saud Kamran and others added 3 commits July 13, 2026 10:37
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.
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.
Distributed loss tests either lived in a separate module disconnected from
each loss's tests, or mocked torch.distributed - which only tests the mock,
not the real collective (the gap that hid lightly-ai#1977 until the mock tests were
reverted in lightly-ai#1980).

Add a torchmetrics-style session-scoped gloo (CPU) process pool, started once
per session and reused, gated behind USE_PYTEST_POOL (default off, not enabled
in CI). Tests opt in with @pytest.mark.DDP + skipif. First consumer: two real
2-rank BarlowTwins tests; the gradient test is a regression test for lightly-ai#1977 and
fails on the raw dist.all_reduce.

The pool uses spawn, whose workers re-import the test module and deadlock under
pytest's default prepend import mode; set --import-mode=importlib in addopts.

Adapted from torchmetrics (Apache-2.0), tests/unittests/conftest.py.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

1 participant