Skip to content

[ROCm] Cap Python CI OpenMP threads - #3632

Open
glen-amd wants to merge 1 commit into
developfrom
glen-amd/rocm-python-ci-omp-cap
Open

glen-amd wants to merge 1 commit into
developfrom
glen-amd/rocm-python-ci-omp-cap

Conversation

@glen-amd

Copy link
Copy Markdown

Ignore defects or problems from the base develop branch, which is out of scope.

AI-assisted implementation summary (OpenAI Codex gpt-6-astra, cross-reviewed by Claude Opus 5)

This PR targets ROCm/pytorch:develop and changes only .ci/pytorch/test.sh.

ROCm CI containers can share a node without CPU isolation, letting each
Python process create an intra-op thread pool sized to the host CPU count.
The fix routes sharded Python tests through test_python and sets a
function-local, exported OMP_NUM_THREADS when ROCm has no existing override.
It uses max(4, nproc / max(4, NUM_TEST_SHARDS)): 24 threads for 192 CPUs and
eight shards. Explicit overrides, existing ARC behavior, and caller thread
settings are preserved.

Validation: shell syntax, all 31 local regression cases, whitespace checks,
and lintrunner 0.12.7 passed with CODEOWNERS_TAXONOMY excluded. That check
reports 15 uncovered paths in the unchanged develop baseline; no ownership
or lint configuration changes are included. The regression harness is
local-only, ignored, and not included in this PR.

Scope and remaining validation: distributed, Inductor, and C++ entry points
remain unchanged, including the distributed-test outlier discussed in the
review. A real ROCm CI/performance canary is still required before merge to
confirm CPU-heavy improvements without GPU-test regressions.

ROCm test containers can share a node without CPU isolation. With OpenMP
unset, PyTorch can create an intra-op pool sized to the node-wide CPU count
in each container, oversubscribing CPUs when test shards run concurrently.

Route Python shards through test_python and set an exported, function-local
OMP_NUM_THREADS for ROCm only when no override is set. Divide nproc by the
greater of four and the shard count, retaining the ARC-style four-thread
floor. Keeping the cap local to non-distributed Python tests preserves the
caller environment and lets distributed launchers retain their defaults.

Test Plan
=========
Ran on the develop-based change using lintrunner 0.12.7:

```bash
lintrunner -a --revision de8e76a --skip CODEOWNERS_TAXONOMY
bash -n .ci/pytorch/test.sh agent_space/check_rocm_omp.sh
bash agent_space/check_rocm_omp.sh
git diff --check de8e76a
git diff --cached --check
```

Syntax, all 31 local regression cases, whitespace checks, and the
non-excluded linters passed. CODEOWNERS_TAXONOMY was excluded because the
full lint run reports 15 uncovered paths in the unchanged develop baseline;
the contributor explicitly directed that existing base defects stay out
of scope. No CODEOWNERS or lint configuration changes are included.

The regression harness is local-only and ignored, not a repository-provided
test or part of this commit. A real ROCm CI/performance canary remains
required before merge; distributed, Inductor, and C++ entry points are
outside this change's scope.

Authored with assistance from OpenAI Codex.
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