Skip to content

Skipping tcp tests, fixing ci#3775

Merged
vfdev-5 merged 3 commits into
pytorch:masterfrom
TahaZahid05:fix-torchrun-ci-v2
Jun 3, 2026
Merged

Skipping tcp tests, fixing ci#3775
vfdev-5 merged 3 commits into
pytorch:masterfrom
TahaZahid05:fix-torchrun-ci-v2

Conversation

@TahaZahid05

Copy link
Copy Markdown
Collaborator

Description:
This PR resolves the CI hangs and provides a clear guide to users on how to migrate away from the legacy tcp:// initialization method

Check list:

  • New tests are added (if a new feature is added)
  • New doc strings: description and/or example code are in RST format
  • Documentation is updated (if required)

@github-actions github-actions Bot added the module: distributed Distributed module label Jun 2, 2026
@TahaZahid05
TahaZahid05 requested a review from vfdev-5 June 3, 2026 00:31
Comment thread tests/ignite/conftest.py Outdated
Comment thread tests/ignite/conftest.py Outdated
@TahaZahid05

Copy link
Copy Markdown
Collaborator Author

@vfdev-5 done.

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 aims to eliminate CI hangs related to legacy tcp://-based distributed initialization by removing tcp:// coverage from the test matrix, switching tests/fixtures to env://, and adding explicit runtime guards that reject tcp:// init methods with a migration message.

Changes:

  • Removed tcp://... from several distributed test parametrizations to stop exercising the legacy path that can hang.
  • Updated distributed test fixtures to use env:// and set MASTER_ADDR/MASTER_PORT explicitly.
  • Added explicit ValueError guards rejecting init_method="tcp://..." in idist.Parallel and the native computation model, plus new tests asserting the error.

Reviewed changes

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

Show a summary per file
File Description
tests/ignite/distributed/utils/test_native.py Removes tcp:// init_method from distributed test parametrization.
tests/ignite/distributed/test_launcher.py Drops tcp:// param case and adds a test asserting tcp:// now errors.
tests/ignite/distributed/comp_models/test_native.py Replaces tcp:// init with TCPStore usage in context tests; removes tcp:// parametrization; adds a new error test.
tests/ignite/conftest.py Switches single-node distributed fixtures from tcp:// to env:// and sets MASTER_ADDR/MASTER_PORT.
ignite/distributed/launcher.py Removes tcp:// example and raises ValueError if init_method starts with tcp://.
ignite/distributed/comp_models/native.py Raises ValueError if init_method starts with tcp:// in native backend creation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/ignite/conftest.py
Comment on lines 233 to 236
free_port = _setup_free_port(local_rank)
os.environ["MASTER_ADDR"] = "localhost"
os.environ["MASTER_PORT"] = str(free_port)

Comment thread tests/ignite/conftest.py
Comment on lines 485 to 490
temp_file = None
free_port = _setup_free_port(local_rank)
init_method = f"tcp://localhost:{free_port}"
init_method = "env://"
os.environ["MASTER_ADDR"] = "localhost"
os.environ["MASTER_PORT"] = str(free_port)

Comment on lines +337 to +338
store = dist.TCPStore("0.0.0.0", 2222, world_size=1, is_master=True)
dist.init_process_group(true_backend, store=store, world_size=1, rank=0)
Comment on lines +362 to +364
is_master = rank == 0
store = dist.TCPStore("0.0.0.0", 2222, world_size=world_size, is_master=is_master)
dist.init_process_group(true_backend, store=store, world_size=world_size, rank=rank)
Comment on lines +450 to +452
is_master = local_rank == 0
store = dist.TCPStore("0.0.0.0", 2222, world_size=world_size, is_master=is_master)
dist.init_process_group("nccl", store=store, world_size=world_size, rank=local_rank)
Comment thread tests/ignite/conftest.py
Comment on lines 259 to +263
free_port = _setup_free_port(local_rank)
init_method = f"tcp://localhost:{free_port}"
init_method = "env://"
temp_file = None
os.environ["MASTER_ADDR"] = "localhost"
os.environ["MASTER_PORT"] = str(free_port)

@vfdev-5 vfdev-5 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @TahaZahid05 !

@vfdev-5
vfdev-5 added this pull request to the merge queue Jun 3, 2026
Merged via the queue into pytorch:master with commit 9ce6a2d Jun 3, 2026
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: distributed Distributed module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants