Skip to content

ci: run distributed pool tests in a dedicated gloo job - #2011

Open
saud5150 wants to merge 1 commit into
lightly-ai:masterfrom
saud5150:ci/enable-distributed-test-pool
Open

ci: run distributed pool tests in a dedicated gloo job#2011
saud5150 wants to merge 1 commit into
lightly-ai:masterfrom
saud5150:ci/enable-distributed-test-pool

Conversation

@saud5150

Copy link
Copy Markdown
Contributor

Part of #1982.

Description

  • My change is breaking

The distributed test pool from #1983 is gated behind USE_PYTEST_POOL, which defaults to off, so its @pytest.mark.DDP tests never run in CI — they only execute when the flag is set manually. This adds a dedicated distributed-tests job that runs the DDP tests on the shared gloo (CPU) pool with USE_PYTEST_POOL=1, so real multi-rank collectives are exercised on every PR instead of relying on mocks (the gap that hid #1977 until #1980).

The flag stays off by default everywhere else, so local runs and the main 3.7/3.12 test matrix are unchanged — only this job enables the pool. It's a separate job (not a step in the existing test job) so a pool issue can't destabilise the main suite, and it runs on Python 3.12, the version I verified on Linux (below).

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.

Before enabling it, I reproduced the CI runner in Docker (python:3.12-slim, CPU torch, gloo) to confirm the spawn path runs green on Linux — Linux defaults multiprocessing to fork, but the pool forces spawn, so it behaves like macOS:

# flag ON — exact CI command form
USE_PYTEST_POOL=1 python -m pytest -s -v --runslow -m DDP tests/loss/
  => 2 passed, 1055 deselected      # both BarlowTwins DDP tests run on the pool

# flag OFF (control)
python -m pytest tests/loss/test_barlow_twins_loss.py
  => 4 passed, 2 skipped            # DDP tests correctly skip

The job's collection scope matches the existing test job (whole repo, --ignore=./lightly/openapi_generated/), which already proves collection works with the full pinned deps.

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.

CI-only change — no public API or docs.

Implications / comments / further issues

A few decisions I'd value your call on:

  • 3.12 only for now (the version I verified on Linux); happy to add 3.7 if you want it covered.
  • Dedicated job vs. a step in the existing 3.12 test job — I chose a separate job for isolation; a step would be smaller but couples pool flakiness to the main suite.
  • If you make this a required check, it's skipped on docs/examples/benchmarks-only PRs (same detect-code-changes gate as test), so it needs the same required-but-skippable handling — related to ci: run required checks for merge groups #2007.

Unrelated side-finding while testing on torch 2.13: torch.distributed.nn.functional.all_reduce (used by the #1980 BarlowTwins fix) now emits a FutureWarning (deprecated for torch.distributed._functional_collectives.all_reduce). Not touched here — flagging as a possible follow-up.

Add a distributed-tests job that runs the @pytest.mark.DDP tests with
USE_PYTEST_POOL=1 on the shared gloo (CPU) process pool. The flag stays
off by default everywhere else, so local runs and the main test matrix
are unchanged; only this job activates the pool.

Kept in its own Python 3.12 job so a pool issue cannot destabilize the
main 3.7/3.12 suite. Verified the spawn/gloo/importlib path runs green on
Linux: both BarlowTwins DDP tests pass with the flag on and skip with it
off.

Part of lightly-ai#1982.
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