Skip to content

Fix PyTorch EP tests sharing a single timeout - #3413

Open
fheinecke wants to merge 1 commit into
NVIDIA:mainfrom
fheinecke:fred/fix-ep-test-timeout-1
Open

Fix PyTorch EP tests sharing a single timeout#3413
fheinecke wants to merge 1 commit into
NVIDIA:mainfrom
fheinecke:fred/fix-ep-test-timeout-1

Conversation

@fheinecke

Copy link
Copy Markdown
Collaborator

Description

This PR added several additional PyTorch EP tests which push the test script over the 210s limit.

I've removed this limit entirely because already has a 10s timeout. Having both is redundant, and causes the tests to collectively fail when the total runtime is > 210s.

This is blocking the 2.19 QA build.

Type of change

  • Documentation change (change only to the documentation, either a fix or a new content)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Infra/Build change
  • Code refactoring

Changes

Please list the changes introduced in this PR:

  • Fix duplicate test timeout causing premature termination and failure of PyTorch EP test suite

Checklist:

  • I have read and followed the contributing guidelines
  • The functionality is complete
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Signed-off-by: Fred Heinecke <fheinecke@nvidia.com>
@fheinecke fheinecke added the 2.19 label Aug 21, 2026
@fheinecke

Copy link
Copy Markdown
Collaborator Author

/te-ci L1 pytorch

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

LGTM

Short timeout so a hang on any rank surfaces fast rather than burning CI time.
The launcher applies its timeout independently to every torchrun pass.
"""
timeout_s = int(os.environ.get("NVTE_TEST_EP_TIMEOUT_S", "180"))

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.

I guess, we should have multiple ep tests share a single torchrun to decrease the overall timings for test. cc: @phu0ngng

@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes the shared Python subprocess timeout so each PyTorch expert-parallel launcher pass can use its own timeout.

  • Updates the test documentation to describe per-pass timeout handling.
  • Leaves the complete launcher pipeline without a fallback bound if timed-out rank processes survive.

Confidence Score: 4/5

The PR should not merge until the launcher retains an overall fallback timeout that can terminate a pipeline left open by surviving rank processes.

The per-pass timeout covers torchrun rather than the complete tee pipeline, while cleanup is unreachable until that pipeline exits and the Python caller no longer provides a fallback deadline.

Files Needing Attention: tests/pytorch/distributed/test_ep.py, tests/pytorch/distributed/run_test_ep.sh

Important Files Changed

Filename Overview
tests/pytorch/distributed/test_ep.py Removes the shared suite timeout as intended, but also removes the only bound covering the launcher pipeline and post-timeout cleanup.

Reviews (1): Last reviewed commit: "Fix PyTorch EP tests sharing a single ti..." | Re-trigger Greptile

Comment thread tests/pytorch/distributed/test_ep.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants