Repository navigation
Add a nightly multi-rank CPU suite - #8806
Merged
Merged
Conversation
cpu-torch-latest advertises a single device, so the per-device gate in tests/unit/common.py skips every world_size > 1 test there. This adds a nightly run that sets LOCAL_SIZE and selects only the multi-rank tests (DS_MULTIRANK_ONLY), which is how the bf16 overflow semantics change from deepspeedai#8638 was found (issue deepspeedai#8805). It is a nightly rather than a pull-request check: the multi-rank half needs ~70-90 minutes on a 4-vCPU runner (measured in deepspeedai#8381), against 39 minutes for the longest current pre-merge job, and a wedged rank would hold up the merge queue. The companion triage workflow reuses ci/nightly_classify.sh and ci/nightly_report.sh with its own issue titles and its own last-green tag, so the modal full-suite tag and its issue dedupe are untouched. Signed-off-by: Guokai Ma <guokai.ma@intel.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Collaborator
Author
|
Note #8805 needs to be fixed for this nightly to pass. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95c20f96b1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Signed-off-by: Ma, Guokai <guokai.ma@intel.com>
GitHub Actions echoes a run: script into the job log before executing it, so the inline sentinel literals were matched by nightly_classify.sh's head -1 ahead of any runtime line, routing every failure as a timeout. Print the sentinel from ci/classify_multirank_exit.py instead, mirroring the modal nightly's controller, and pin the contract with tests. Signed-off-by: Ma, Guokai <guokai.ma@intel.com>
Inside a POSIX bracket expression \t is backslash-and-t, not a tab, so the class excluded the letter t and matched no test_*.py path at all; the report job then aborted before nightly_report.sh could run. Use the [[:space:]] class, and turn the FAILED prefix into the tests/ prefix instead of prepending to it. Signed-off-by: Ma, Guokai <guokai.ma@intel.com>
The selector in tests/conftest.py runs only in the nightly multi-rank workflow, so CI never exercised it. Drive the real conftest in a sandbox and pin the observable contract: the gate is a no-op when disabled, only world_size>1 dist tests survive when enabled, and a method mark overrides the class world_size in both directions. Signed-off-by: Ma, Guokai <guokai.ma@intel.com>
Both runs on master resolved the concurrency group to the same value, so a debug dispatch with cancel-in-progress killed the scheduled nightly mid-run and triage reported the cancellation as a failure. Include the event name in the group, as modal-torch-latest already does. Signed-off-by: Ma, Guokai <guokai.ma@intel.com>
The second phase's exit code overwrote the first's, so the same pair of failures classified differently depending on phase order and a timeout could be silently downgraded to a test failure. Keep both exit codes and classify 124 first: a timeout means the budget ran out and more failures may remain undiscovered. Signed-off-by: Ma, Guokai <guokai.ma@intel.com>
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Oct 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds the
world_sizeaxis to CI, which the current suite does not cover:.github/workflows/cpu-torch-multi-latest.yml.github/workflows/cpu-multi-triage.ymlci/nightly_classify.sh/ci/nightly_report.shprotocoltests/conftest.pyDS_MULTIRANK_ONLYcollection hook the workflow selects with (required).github/workflows/TEST_SELECTION.mdWhy
cpu-torch-latestadvertises a single device, so the per-device gate intests/unit/common.pyskips everyworld_size > 1test on that runner. Themulti-rank path is therefore only exercised on GPU machines, and issues that show up
there only in a multi-rank configuration go unnoticed.
A recent example: PR #8638 changed the overflow semantics so that non-finite
group norms force an optimizer step to be skipped, bypassing
bf16.check_grad_overflow: False.TestZeROBFloat16::test_ignore_grad_overflowisworld_size = 2, so it is collected and skipped by the CPU CI and by any run withone device; it fails as soon as two ranks execute. Reported as issue #8805.
Why a nightly rather than a pre-merge check
The selection is cheap to state and expensive to run (measured in #8381, 4-vCPU
runner):
sequential-marked phasecpu-torch-latest)modal-torch-latest)Putting it in the pre-merge path would roughly triple the critical path of every PR,
and a single wedged rank would block the merge queue (the suite's known hang — the
shm op segfaulting on hosts without AVX512 — is fixed by #8701, but the suite is the
most expensive one to re-run).
What it runs
LOCAL_SIZE=4so the gate admitsworld_size <= 4;DS_MULTIRANK_ONLY=1deselectsthe single-rank tests that
cpu-torch-latestalready guards.timeout 120m, plus asequential-marked phase,timeout 25m(theparallel phase deselects those by default, so they would otherwise never run here).
cpu-torch-latest, so both runs agree onthe environment.
DS_CI_FAILURE_CLASS=timeout|testfor triage; a runthat dies before that step is reported as unclassified rather than guessed.
Triage is a separate workflow with its own issue titles and its own
nightly-cpu-multi-last-greentag, sonightly-last-greenkeeps meaning "the lastrevision the full GPU suite verified" and the modal issue dedupe is untouched.
Validation
pre-commitclean.cpu-torch-multi-latest: GitHub only exposesworkflow_dispatchforworkflows present on the default branch, so the first manual dispatch has to happen
after this merges. Happy to run it then and follow up with the numbers.