Skip to content

Commit a22c41f

Browse files
authored
Merge branch 'master' into peng-auto-ep-optimizer-fix-v1
2 parents eea5342 + cbd303e commit a22c41f

76 files changed

Lines changed: 4341 additions & 293 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/TEST_SELECTION.md

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,9 @@ one config — see [Adding a workflow](#adding-a-new-workflow).
2525
the import graph from your changed files to the impacted tests, and writes the
2626
list to `ci/.test_selection/test_list.txt`.
2727
- The trusted controller validates that list, then fetches, installs, and tests
28-
the exact candidate SHA inside a no-secret Modal Sandbox. `push` to `master`
29-
and manual runs always run everything.
28+
the exact candidate SHA inside a no-secret Modal Sandbox. The Sandbox runs
29+
only for merge queue entries, `push` to `master` (always the full suite), and
30+
manual runs — a plain PR event never spends Modal quota.
3031
- It is **fail-safe**: anything it can't reason about safely → run the *full* suite.
3132
It never silently runs *fewer* tests than reality.
3233
- Preview locally:
@@ -63,12 +64,12 @@ The design is a small, self-contained take on HuggingFace `transformers`'
6364
### Job flow
6465

6566
```
66-
pull_request_target / push / workflow_dispatch
67+
pull_request_target / merge_group / push / workflow_dispatch
6768
6869
┌─────────────┴─────────────┐
6970
│ collect-tests │ (no secrets)
7071
│ checkout exact base SHA │
71-
public-fetch exact PR SHA
72+
│ public-fetch exact head SHA│
7273
│ self-test the fetcher │
7374
│ parse candidate as data │
7475
│ → mode (all|subset|none) │
@@ -83,7 +84,7 @@ The design is a small, self-contained take on HuggingFace `transformers`'
8384
│ validate mode/list + SHA │
8485
│ create one L40S:2 Sandbox │
8586
│ no secrets or mounts │
86-
│ fetch exact PR SHA
87+
│ fetch exact head SHA │
8788
│ install + run pytest │
8889
│ terminate/observe Sandbox │
8990
└────────────────────────────┘
@@ -96,6 +97,14 @@ The design is a small, self-contained take on HuggingFace `transformers`'
9697
- **`subset`**`deploy` runs pytest on exactly the impacted files.
9798
- **`all`**`deploy` runs the whole scope (`tests/unit/v1`).
9899

100+
Independent of `mode`, `deploy` is also skipped on `pull_request_target` runs,
101+
so pushing to a PR never spends Modal quota — the `collect-tests` summary still
102+
previews what the queue will run. The Sandbox actually executes on
103+
`merge_group` (the merged tree, gating the merge), `push` to `master`, and
104+
`workflow_dispatch`. On `merge_group` the candidate is the merge-group commit
105+
in the base repository, diffed against the queue's base SHA, so the selection
106+
covers exactly what the entry would introduce.
107+
99108

100109
## How a decision is made
101110

@@ -280,6 +289,17 @@ context and the `deploy` controller authenticates to Modal. The trust boundary i
280289
mitigation, **not** the trust boundary. Exact trusted base code plus Sandbox
281290
isolation is the primary protection.
282291

292+
**`merge_group` runs are a separate trust context.** GitHub runs a queued
293+
entry's workflows from the merge-group commit — the PR's merged tree — so the
294+
workflow YAML itself is candidate-controlled. The jobs restore the repo's
295+
invariant by resolving every GitHub-side checkout to the trusted base revision
296+
(`merge_group.base_sha`): the selector, the controller, and the accelerate
297+
launcher all come from master there, and the merged candidate enters only as
298+
validated git data (fetched by exact SHA into a separate root) or inside the
299+
no-secret Modal Sandbox. The residual exposure — a queued PR rewriting the
300+
workflow YAML itself to echo secrets — is inherent to GitHub's merge queue;
301+
review PRs that touch `.github/workflows/*` with that in mind.
302+
283303
> **Consequence:** changes to `ci/*` (including `tests_fetcher.py` itself) take
284304
> effect under `pull_request_target` only after they're **merged**. A PR that
285305
> changes this launcher cannot prove its new PR-triggered end-to-end path by
@@ -339,4 +359,6 @@ are still publicly reachable. Do not replace this with a moving branch fallback.
339359
**Why didn't this PR exercise its new `pull_request_target` controller?**
340360
GitHub intentionally runs that event's workflow from the trusted base. The new
341361
controller becomes the trusted code only after merge; before then, rely on the
342-
focused static/unit evidence described in the security model.
362+
focused static/unit evidence described in the security model. (The merge-queue
363+
run *does* use the PR's merged controller, so `ci/*` changes are first
364+
exercised live there.)

.github/workflows/modal-accelerate.yml

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ name: modal-accelerate
1212
# collect-tests job to do the filtering for us so that the job can be skipped and satisfy the
1313
# Required status for PRs to pass.
1414
#
15+
# Modal GPUs are a limited quota, so the deploy job only runs for merge queue entries, pushes to
16+
# master, and manual runs. Plain PR events keep the job skipped (which still satisfies the
17+
# Required status); the merge queue re-runs it on the merged tree. On merge queue runs the
18+
# launcher (this checkout) stays on trusted master and the merged candidate is fetched as data,
19+
# so queued PR code never executes on this runner next to the Modal/HF tokens.
1520

1621

1722
on:
@@ -35,6 +40,14 @@ on:
3540
branches:
3641
- master
3742

43+
# Required checks must also trigger on merge queue entries, otherwise the
44+
# merge fails waiting for a status that is never reported. This is the only
45+
# PR-related event that spends Modal quota.
46+
merge_group:
47+
types: [checks_requested]
48+
branches:
49+
- master
50+
3851
concurrency:
3952
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
4053
cancel-in-progress: true
@@ -55,6 +68,8 @@ jobs:
5568
with:
5669
lfs: true
5770

71+
# paths-filter v4.0.1+ covers merge_group natively (it diffs the event's
72+
# base/head commits), so no extra inputs are needed here.
5873
- name: Filter changed files
5974
uses: dorny/paths-filter@v4
6075
id: filter
@@ -81,13 +96,35 @@ jobs:
8196
# and it too is then updated at https://github.com/deepspeedai/deepspeed/settings/secrets/actions
8297
HF_TOKEN: ${{ secrets.HF_TOKEN }}
8398

84-
if: needs.collect-tests.outputs.deepspeed == 'true'
99+
# Plain PR events keep this job skipped so they never spend Modal quota (a
100+
# skipped dependent job still satisfies the Required status); the merge
101+
# queue re-runs it on the merged tree.
102+
if: needs.collect-tests.outputs.deepspeed == 'true' && github.event_name != 'pull_request_target'
85103
steps:
86104
- name: Checkout Repository
87105
uses: actions/checkout@v7
88106
with:
107+
# Trusted base revision on merge queue runs (master otherwise), so
108+
# ci/accelerate.py holding the Modal/HF tokens is never queued-PR code.
109+
ref: ${{ github.event.merge_group.base_sha || github.sha }}
89110
lfs: true
90111

112+
- name: Fetch merge queue candidate tree
113+
if: github.event_name == 'merge_group'
114+
env:
115+
HEAD_REPOSITORY: ${{ github.repository }}
116+
HEAD_SHA: ${{ github.sha }}
117+
BASE_REPOSITORY: ${{ github.repository }}
118+
BASE_SHA: ${{ github.event.merge_group.base_sha }}
119+
CANDIDATE_ROOT: ${{ runner.temp }}/deepspeed-candidate
120+
run: |
121+
python3 ci/torch_latest.py checkout-candidate \
122+
--head-repository "$HEAD_REPOSITORY" \
123+
--head-sha "$HEAD_SHA" \
124+
--base-repository "$BASE_REPOSITORY" \
125+
--base-sha "$BASE_SHA" \
126+
--destination "$CANDIDATE_ROOT"
127+
91128
- name: Install Python
92129
uses: actions/setup-python@v7
93130
with:
@@ -100,5 +137,9 @@ jobs:
100137
uv pip install --system modal
101138
102139
- name: Run tests
140+
env:
141+
# Tree under test: the fetched merge-group candidate on queue runs,
142+
# empty (this checkout) everywhere else.
143+
DS_CI_CANDIDATE_ROOT: ${{ github.event_name == 'merge_group' && format('{0}/deepspeed-candidate', runner.temp) || '' }}
103144
run: |
104145
modal run -m ci.accelerate

.github/workflows/modal-torch-latest.yml

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ name: modal-torch-latest
33
# This Required workflow selects tests on a no-secret GitHub runner, then tests
44
# the exact candidate SHA inside a no-secret Modal Sandbox. Under
55
# pull_request_target, every GitHub-side checkout and Python entrypoint comes
6-
# from the trusted base commit. See .github/workflows/TEST_SELECTION.md.
6+
# from the trusted base commit; merge queue runs resolve that trusted base from
7+
# merge_group.base_sha. The Sandbox only runs for merge queue entries, pushes to
8+
# master, and manual runs, so plain PR events never spend Modal quota. See
9+
# .github/workflows/TEST_SELECTION.md.
710

811
on:
912
workflow_dispatch:
@@ -42,6 +45,14 @@ on:
4245
branches:
4346
- master
4447

48+
# Required checks must also trigger on merge queue entries, otherwise the
49+
# merge fails waiting for a status that is never reported. This is the only
50+
# PR-related event that spends Modal quota.
51+
merge_group:
52+
types: [checks_requested]
53+
branches:
54+
- master
55+
4556
permissions:
4657
contents: read
4758

@@ -63,19 +74,21 @@ jobs:
6374
- name: Checkout trusted workflow code
6475
uses: actions/checkout@v7
6576
with:
66-
ref: ${{ github.event.pull_request.base.sha || github.sha }}
77+
# Trusted base revision for every event: the merge-group commit must
78+
# never control the selector that runs on this GitHub runner.
79+
ref: ${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha || github.sha }}
6780
fetch-depth: 1
6881
persist-credentials: false
6982
lfs: false
7083
submodules: false
7184

7285
- name: Fetch candidate tree as data
73-
if: github.event_name == 'pull_request_target'
86+
if: github.event_name == 'pull_request_target' || github.event_name == 'merge_group'
7487
env:
75-
HEAD_REPOSITORY: ${{ github.event.pull_request.head.repo.full_name }}
76-
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
77-
BASE_REPOSITORY: ${{ github.event.pull_request.base.repo.full_name }}
78-
BASE_SHA: ${{ github.event.pull_request.base.sha }}
88+
HEAD_REPOSITORY: ${{ github.event.pull_request.head.repo.full_name || github.repository }}
89+
HEAD_SHA: ${{ github.event.pull_request.head.sha || github.sha }}
90+
BASE_REPOSITORY: ${{ github.event.pull_request.base.repo.full_name || github.repository }}
91+
BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }}
7992
CANDIDATE_ROOT: ${{ runner.temp }}/deepspeed-candidate
8093
run: |
8194
python3 ci/torch_latest.py checkout-candidate \
@@ -94,7 +107,7 @@ jobs:
94107
EVENT_NAME: ${{ github.event_name }}
95108
CANDIDATE_ROOT: ${{ runner.temp }}/deepspeed-candidate
96109
run: |
97-
if [ "$EVENT_NAME" = "pull_request_target" ]; then
110+
if [ "$EVENT_NAME" = "pull_request_target" ] || [ "$EVENT_NAME" = "merge_group" ]; then
98111
python3 ci/tests_fetcher.py \
99112
--workflow modal-torch-latest \
100113
--repo-root "$CANDIDATE_ROOT" \
@@ -125,14 +138,17 @@ jobs:
125138
deploy:
126139
name: modal-torch-latest / DeepSpeedAI CI
127140
runs-on: ubuntu-latest
128-
timeout-minutes: 75
141+
timeout-minutes: 105
129142
permissions:
130143
contents: read
131144
needs: collect-tests
132145

133146
# Preserve the Required check on failures and no-test selections. A selector
134147
# failure enters this job and fails explicitly; mode=none skips the Sandbox.
135-
if: ${{ !cancelled() && (needs.collect-tests.result != 'success' || needs.collect-tests.outputs.mode != 'none') }}
148+
# pull_request_target runs skip the Sandbox entirely (a skipped dependent
149+
# job still satisfies the Required check), so the tests gate the merge queue
150+
# entry instead of every PR update.
151+
if: ${{ !cancelled() && (needs.collect-tests.result != 'success' || (needs.collect-tests.outputs.mode != 'none' && github.event_name != 'pull_request_target')) }}
136152
steps:
137153
- name: Fail if test selection failed
138154
if: needs.collect-tests.result != 'success'
@@ -141,7 +157,9 @@ jobs:
141157
- name: Checkout trusted controller code
142158
uses: actions/checkout@v7
143159
with:
144-
ref: ${{ github.event.pull_request.base.sha || github.sha }}
160+
# Trusted base revision for every event: the merge-group commit is
161+
# only ever fetched by exact SHA inside the no-secret Sandbox.
162+
ref: ${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha || github.sha }}
145163
fetch-depth: 1
146164
persist-credentials: false
147165
lfs: false

.github/workflows/nv-flash-attn.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ on:
55
pull_request:
66
paths:
77
- 'deepspeed/sequence/**'
8-
- 'tests/unit/sequence_parallelism/**'
8+
- 'tests/unit/v1/sequence_parallelism/**'
99
- '.github/workflows/nv-flash-attn.yml'
1010
schedule:
1111
- cron: "0 0 * * *"
@@ -57,7 +57,7 @@ jobs:
5757
run: |
5858
unset TORCH_CUDA_ARCH_LIST # only jit compile for current arch
5959
cd tests
60-
python -m pytest --color=yes --durations=0 --verbose -rF unit/sequence_parallelism/test_ulysses.py --torch_ver="2.6" --cuda_ver="12"
60+
python -m pytest --color=yes --durations=0 --verbose -rF unit/v1/sequence_parallelism/test_ulysses.py --torch_ver="2.6" --cuda_ver="12"
6161
- name: Open GitHub issue if nightly CI fails
6262
if: ${{ failure() && (github.event_name == 'schedule') }}
6363
uses: JasonEtco/create-an-issue@v2

AGENTS.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,16 @@
2525
- Comments should explain **why**, not **what**. Describe the purpose and reasoning, not the mechanics that the code already shows.
2626
- New features must include corresponding tests and documentation updates.
2727

28+
### Test discipline
29+
30+
- Tests verify contracts, not implementations: a test is well-formed only if a different correct implementation of the same contract passes it.
31+
- Before writing a test, name the concrete incorrect behavior it would catch; if you cannot name one, do not write it.
32+
- Assert on observable outcomes through public/stable interfaces; do not assert private method return values or exact internal strings unless pinning a specific fixed bug (justify in a comment).
33+
- Anchor to an external oracle or an independently derived reference instead of re-implementing the logic under test.
34+
- Mocks must stand in for a collaborator's documented contract (schema, protocol), never for internals of the module under test.
35+
- Changes that affect the external contract at the training-loop or inference level require integration tests, not just unit tests (e.g. a minimal training loop with `SimpleModel`).
36+
- Integration tests must be executed on actual devices, not merely written: report the execution hardware spec and results in the PR.
37+
2838
## Tool Caveats
2939

3040
### Edit tool auto-formatter

CLAUDE.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,16 @@
2525
- Comments should explain **why**, not **what**. Describe the purpose and reasoning, not the mechanics that the code already shows.
2626
- New features must include corresponding tests and documentation updates.
2727

28+
### Test discipline
29+
30+
- Tests verify contracts, not implementations: a test is well-formed only if a different correct implementation of the same contract passes it.
31+
- Before writing a test, name the concrete incorrect behavior it would catch; if you cannot name one, do not write it.
32+
- Assert on observable outcomes through public/stable interfaces; do not assert private method return values or exact internal strings unless pinning a specific fixed bug (justify in a comment).
33+
- Anchor to an external oracle or an independently derived reference instead of re-implementing the logic under test.
34+
- Mocks must stand in for a collaborator's documented contract (schema, protocol), never for internals of the module under test.
35+
- Changes that affect the external contract at the training-loop or inference level require integration tests, not just unit tests (e.g. a minimal training loop with `SimpleModel`).
36+
- Integration tests must be executed on actual devices, not merely written: report the execution hardware spec and results in the PR.
37+
2838
## Tool Caveats
2939

3040
### Edit tool auto-formatter

CONTRIBUTING.md

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,12 @@ make test
5353

5454
### Diff-based CI test selection
5555
Some GPU CI workflows (currently `modal-torch-latest`, which runs `tests/unit/v1/`)
56-
don't run the whole suite on every PR. Instead, `ci/tests_fetcher.py` looks at the
57-
files your PR changes, builds an import graph over `deepspeed/` and the `unit` test
58-
helpers, and runs only the tests that could be affected. This keeps CI fast without
59-
losing coverage (`push` to `master` always runs everything). The full design — and
56+
run their modal tests on the merge queue entry instead of on every PR push, to
57+
conserve Modal GPU quota. `ci/tests_fetcher.py` looks at the files your PR
58+
changes, builds an import graph over `deepspeed/` and the `unit` test helpers,
59+
and selects only the tests that could be affected. The selection is previewed on
60+
the PR by a cheap no-secret job and executed once the PR enters the merge queue
61+
(`push` to `master` always runs everything). The full design — and
6062
how to drive and extend it — is in
6163
[`.github/workflows/TEST_SELECTION.md`](.github/workflows/TEST_SELECTION.md).
6264

@@ -91,9 +93,11 @@ cat ci/.test_selection/test_list.txt
9193
python ci/tests_fetcher.py --base origin/master --explain
9294
```
9395

94-
> Note: under `pull_request_target` the `deploy` job runs the PR's `deepspeed/` and
95-
> `tests/` but restores `ci/` from the base branch (the CI scripts hold the modal/HF
96-
> secrets). So changes to `ci/*` only take effect once merged — validate them via a
96+
> Note: plain PR events never start the modal `deploy` job (it stays skipped to
97+
> conserve GPU quota), and under `pull_request_target` GitHub runs the base branch's
98+
> CI scripts anyway. Merge queue runs also use trusted master for the CI scripts —
99+
> your merged tree is only fetched as the candidate under test — so changes to
100+
> `ci/*` take effect once **merged**; validate them before that via a
97101
> `pull_request`-triggered run or the `modal` CLI.
98102
99103
### Model Tests

ci/accelerate.py

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,26 +3,33 @@
33

44
# DeepSpeed Team
55

6+
import os
67
from pathlib import Path
78

89
import modal
910

1011
ROOT_PATH = Path(__file__).parents[1]
1112

13+
# Tree under test. On merge queue runs this launcher is checked out from
14+
# trusted master, so the merged candidate is fetched separately and pointed at
15+
# here. Remotely the env var is unset and this module is materialized inside
16+
# the image at /root, where it must keep resolving to ROOT_PATH.
17+
CANDIDATE_PATH = Path(os.environ.get("DS_CI_CANDIDATE_ROOT") or ROOT_PATH)
18+
1219
# yapf: disable
1320
image = (modal.Image
1421
.from_registry("pytorch/pytorch:2.9.1-cuda12.8-cudnn9-devel", add_python="3.10")
1522
.apt_install("git")
1623
.pip_install("uv")
1724
# uv_pip_install already includes --compile-bytecode
1825
.uv_pip_install("datasets==3.6.0", extra_options="--system")
19-
.pip_install_from_requirements(ROOT_PATH / "requirements/requirements.txt", gpu="any")
20-
.pip_install_from_requirements(ROOT_PATH / "requirements/requirements-dev.txt", gpu="any")
21-
.add_local_dir(ROOT_PATH , remote_path="/root/", copy=True)
26+
.pip_install_from_requirements(CANDIDATE_PATH / "requirements/requirements.txt", gpu="any")
27+
.pip_install_from_requirements(CANDIDATE_PATH / "requirements/requirements-dev.txt", gpu="any")
28+
.add_local_dir(CANDIDATE_PATH, remote_path="/root/", copy=True)
2229
.run_commands("pip install /root")
23-
.add_local_dir(ROOT_PATH / "accelerator", remote_path="/root/deepspeed/accelerator")
24-
.add_local_dir(ROOT_PATH / "csrc", remote_path="/root/deepspeed/ops/csrc")
25-
.add_local_dir(ROOT_PATH / "op_builder", remote_path="/root/deepspeed/ops/op_builder")
30+
.add_local_dir(CANDIDATE_PATH / "accelerator", remote_path="/root/deepspeed/accelerator")
31+
.add_local_dir(CANDIDATE_PATH / "csrc", remote_path="/root/deepspeed/ops/csrc")
32+
.add_local_dir(CANDIDATE_PATH / "op_builder", remote_path="/root/deepspeed/ops/op_builder")
2633
)
2734

2835
app = modal.App("deepspeedai-accelerate-ci", image=image)

0 commit comments

Comments
 (0)