Fix PyTorch EP tests sharing a single timeout - #3413
Conversation
Signed-off-by: Fred Heinecke <fheinecke@nvidia.com>
|
/te-ci L1 pytorch |
| 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")) |
There was a problem hiding this comment.
I guess, we should have multiple ep tests share a single torchrun to decrease the overall timings for test. cc: @phu0ngng
Greptile SummaryThis PR removes the shared Python subprocess timeout so each PyTorch expert-parallel launcher pass can use its own timeout.
Confidence Score: 4/5The 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
Reviews (1): Last reviewed commit: "Fix PyTorch EP tests sharing a single ti..." | Re-trigger Greptile |
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
Changes
Please list the changes introduced in this PR:
Checklist: