8000 [Distributed][CI] Rework continuous TestCase by kwen2501 · Pull Request #153653 · pytorch/pytorch · GitHub
[go: up one dir, main page]

Skip to content

[Distributed][CI] Rework continuous TestCase #153653

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: gh/kwen2501/153/base
Choose a base branch
from

Conversation

kwen2501
Copy link
Contributor
@kwen2501 kwen2501 commented May 15, 2025

Stack from ghstack (oldest at bottom):

  1. Reworked MultiProcContinousTest to spawn processes during setUpClass instead of main (so that we can support multiple TestClass'es in one file).

  2. The child processes are now an infinite loop, monitoring test IDs passed from main process via a task queue. Reciprocally, the child processes inform the main process completion of a test via a completion queue.

  3. Added a test template.

cc @H-Huang @awgu @wanchaol @fegin @fduwjj @wz337 @wconstab @d4l3k

[ghstack-poisoned]
Copy link
pytorch-bot bot commented May 15, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/153653

Note: Links to docs will display an error until the docs builds have been completed.

❌ 10 New Failures

As of commit 6f6d7de with merge base fa85434 (image):

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

kwen2501 added a commit that referenced this pull request May 15, 2025
Move rdvz file clean-up

Reduce test cases to avoid OOM


ghstack-source-id: 1f5be60
Pull-Request-resolved: #153653
@pytorch-bot pytorch-bot bot added oncall: distributed Add this issue/PR to distributed oncall triage queue release notes: distributed (c10d) release notes category labels May 15, 2025
[ghstack-poisoned]
kwen2501 added a commit that referenced this pull request May 16, 2025
Move rdvz file clean-up

Reduce test cases to avoid OOM

ghstack-source-id: 220f570
Pull-Request-resolved: #153653

Allow spawn processes per MultiProcContTest class
[ghstack-poisoned]
@kwen2501 kwen2501 changed the title [SymmMem] Speed up tests [Distributed][CI] Rework continuous TestCase May 16, 2025
@kwen2501 kwen2501 requested review from fduwjj and d4l3k and removed request for fduwjj May 16, 2025 01:49
Copy link
Member
@d4l3k d4l3k left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor
@fegin fegin left a comment

Choose a reason for hiding this comment

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

LGTM, but the tests failures are real due to Python 3.10 syntax (Pytorch still supports 3.9).

# Rendezvous file
rdvz_file: Optional[str] = None
# timeout configured per class
timeout: timedelta = timedelta(seconds=120)

@classmethod
@abc.abstractmethod
def backend_str(cls) -> str:
def backend_str(cls) -> str | None:
Copy link
Contributor

Choose a reason for hiding this comment

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

PyTorch is still using Python 3.9, which doesn't support | (this supports in Python 3.10)

Copy link
Contributor
@fduwjj fduwjj left a comment

Choose a reason for hiding this comment

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

unblock

# number of test processes
world_size: int = 2
world_size: int = -2 # unset state
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we changing this to -2?


@classmethod
def _run_test_given_id(cls, test_id: str, **kwargs) -> None:
# self.id() == e.g. '__main__.TestDistributed.TestAdditive.test_get_rank'
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: remove?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
oncall: distributed Add this issue/PR to distributed oncall triage queue release notes: distributed (c10d) release notes category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0