Skip to content

surface comm overlap rank errors - #3384

Open
sudhakarsingh27 wants to merge 4 commits into
NVIDIA:mainfrom
sudhakarsingh27:sudhakars/surface-comm-overlap-rank-errors
Open

surface comm overlap rank errors#3384
sudhakarsingh27 wants to merge 4 commits into
NVIDIA:mainfrom
sudhakarsingh27:sudhakars/surface-comm-overlap-rank-errors

Conversation

@sudhakarsingh27

@sudhakarsingh27 sudhakarsingh27 commented Aug 14, 2026

Copy link
Copy Markdown
Member

Summary

  • Surface rank-local comm/GEMM overlap failures with @record and bounded subprocess stdout/stderr so the real per-rank exception is visible.
  • Apply deterministic and backend overrides through a copied child environment so one parametrized subprocess cannot affect later tests.
  • Keep FA4 out of the L1 PyTorch distributed suite because it validates distributed behavior; dedicated attention-backend suites own FA4 coverage.

sudhakarsingh27 and others added 3 commits August 14, 2026 13:41
Comm-overlap launchers captured child stdout and stderr but raised only stderr, which could leave the originating rank context out of pytest and JUnit failures. Preserve the existing result predicates while attaching bounded tails of both streams, and record layer-worker exceptions so torchrun can report the rank-local traceback.

Signed-off-by: Sudhakar Singh <sudhakars@nvidia.com>
The launcher mutated os.environ and then called os.unsetenv, which leaves Python's environment mapping unchanged. Conditional backend flags could therefore leak into later parameterized children. Build a child-only environment instead so each launch gets its intended overrides while preserving the parent process environment.

Signed-off-by: Sudhakar Singh <sudhakars@nvidia.com>
@greptile-apps

greptile-apps Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR improves failure reporting for distributed communication-overlap tests and isolates subprocess-specific environment overrides.

  • Records rank-level exceptions in the distributed layer runner.
  • Centralizes subprocess result validation with bounded stdout and stderr diagnostics.
  • Prevents test-specific environment settings from leaking between parametrized cases.
  • Disables FlashAttention v4 at the distributed QA suite boundary.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains within the eligible follow-up-review scope.

No blocking failure remains.

Important Files Changed

Filename Overview
qa/L1_pytorch_distributed_unittest/test.sh Moves the FlashAttention v4 disablement to a suite-wide export and removes redundant per-command assignments.
tests/pytorch/distributed/run_layer_with_overlap.py Adds Torch Elastic exception recording so distributed rank failures provide structured diagnostics.
tests/pytorch/distributed/test_comm_gemm_overlap.py Adds centralized subprocess validation, text-mode output capture, bounded diagnostics, and isolated child-process environments.

Reviews (2): Last reviewed commit: "Keep FA4 out of L1 distributed tests" | Re-trigger Greptile

@sudhakarsingh27 sudhakarsingh27 changed the title Sudhakars/surface comm overlap rank errors surface comm overlap rank errors Aug 15, 2026
The moving PyTorch image installs FA4 by default, implicitly expanding an L1 suite that historically covered earlier attention backends. Export the FA4 selector at the suite boundary so every pytest and torchrun child retains the intended backend scope while dedicated attention suites own FA4 coverage.

Signed-off-by: Sudhakar Singh <sudhakars@nvidia.com>
@sudhakarsingh27

Copy link
Copy Markdown
Member Author

/te-ci pytorch L1

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