Repository navigation
[OMNIML-6181] [11/11] Share one Megatron worker pool across the gpu_megatron modules - #2855
Open
shengliangxu wants to merge 3 commits into
Open
shengliangxu wants to merge 3 commits into
shengliangxu wants to merge 3 commits into
Conversation
shengliangxu
added this pull request to stack #2856
October 17, 2026 01:41
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Contributor
This was referenced Oct 17, 2026
[OMNIML-6179] [5/11] Make the ONNX tests cheaper: CPU-only ones to the unit lane, small models
#2849
Open
Collaborator
Author
|
/ok to test 3c4611f |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## shengliangx/ci-9-torch-onnx-lane #2855 +/- ##
=================================================================
Coverage 70.08% 70.08%
=================================================================
Files 659 659
Lines 74081 74081
=================================================================
+ Hits 51917 51918 +1
+ Misses 22164 22163 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Collaborator
Author
|
/ok to test 9f2fca6 |
Collaborator
Author
|
/ok to test ca6126d |
shengliangxu
removed this pull request from stack #2856
October 17, 2026 06:58
shengliangxu
force-pushed
the
shengliangx/ci-11-megatron-session-pool
branch
from
October 17, 2026 07:02
ca6126d to
dee192a
Compare
shengliangxu
added this pull request to stack #2859
October 17, 2026 07:02
Collaborator
Author
|
/ok to test dee192a |
shengliangxu
force-pushed
the
shengliangx/ci-11-megatron-session-pool
branch
from
October 18, 2026 23:15
dee192a to
adfbd48
Compare
shengliangxu
marked this pull request as ready for review
October 19, 2026 03:45
shengliangxu
force-pushed
the
shengliangx/ci-11-megatron-session-pool
branch
2 times, most recently
from
October 20, 2026 01:16
1812f62 to
37ef132
Compare
shengliangxu
force-pushed
the
shengliangx/ci-11-megatron-session-pool
branch
from
October 20, 2026 18:40
37ef132 to
038eab6
Compare
shengliangxu
force-pushed
the
shengliangx/ci-11-megatron-session-pool
branch
2 times, most recently
from
October 20, 2026 19:05
f9cbf64 to
39d221e
Compare
shengliangxu
force-pushed
the
shengliangx/ci-11-megatron-session-pool
branch
from
October 20, 2026 19:11
39d221e to
1c3788b
Compare
shengliangxu
force-pushed
the
shengliangx/ci-11-megatron-session-pool
branch
from
October 20, 2026 19:17
1c3788b to
5cd143c
Compare
shengliangxu
added a commit
that referenced
this pull request
Oct 20, 2026
…#2845) ### What does this PR do? Type of change: CI (unit-test runtime) The unit `linux` job gates every GPU and example workflow (their pr-gate jobs wait for it), and its tests are CPU-bound but run serially. This runs them under pytest-xdist on the runner's four vCPUs. The `linux` job and the multi-version matrix get `PYTEST_ADDOPTS=-n 4 --dist loadfile` and `OMP_NUM_THREADS=1`: one worker per vCPU (`-n auto` counts physical cores), whole files per worker, and one thread per worker, because with torch's default threads the runner is oversubscribed and slower than serial (1,481 s and 41 failures in my local runs). What else changes so the suite passes under xdist: - `noxfile.py`: the second pytest process that runs the `manual` tests clears `PYTEST_ADDOPTS`, so it stays serial. Those tests leak global state into each other, and a worker would run them in some order. - `tests/_test_utils/torch/quantization/onnx_export.py`: `TEST_MODELS` was a set of classes, whose iteration order differs between processes, so workers collected different test ids. It is a tuple now. - `test_hf_dflash2.py::test_overfits_a_single_batch` takes about 30 s on one thread on main (53 s before #2836 halved its loop to 30 steps; that version hit the 60 s default call timeout in the nightly on main on 2026-10-20). Its tiny model is built in bf16, and a CPU without native bf16 support emulates bf16 matmuls (about 0.9 s per step, 0.1 s in float32). The model is float32 now, which takes the test to about 4 s on one thread, so it needs no timeout override; the loop and both assertions are unchanged. - `test_full_algorithms_vision.py::test_search_constraints` assigned `ConstraintsFunc._get_true_latency` and never restored it. Serial runs never noticed, because `test_evaluate_constraints.py` sorts first. Under xdist a worker can get the files in the other order, and the Python 3.10 and 3.14 legs failed two tests after it. It uses `monkeypatch` now. - `tests/conftest.py`: the unit per-test cap is 120 s on every platform. `test_dist.py::test_data_parallel` takes 8-15 s serial and 25-35 s with four workers sharing four vCPUs, and went over 60 s on the slowest runner. A hung test is still reported, after 2 min instead of 1. Why: on current main the `linux` job has a median of 12.9 min (27 runs of other PRs since #2823 merged), and a pull request's GPU run waits a median 14.5 min for it (125 runs of 33 PRs). ### Time improvement (measured in CI) Unit `linux` job: serial on current main (27 runs of other PRs since #2823 merged) against the runs of this series with `-n 4`. | | before (serial) | with this PR | |---|---|---| | pytest | 630 s median (355-709 s) | 479 s in this PR's own run; 294 s median over the 11 runs of the series (166-479 s) | | whole job | 12.9 min median (8.8-14.8) | 10.9 min in this PR's own run; 7.0 min median over the 11 runs (5.0-10.9) | | unit workflow: `linux`, then the multi-version and Windows legs | 24.1 min median (12 runs) | 17.2 min median (the 7 runs that ran every leg) | - The runs of #2846 and above also carry the sysmon core of #2846 (about 22% off the unit suite on its own, measured locally), so the 294 s median is xdist plus sysmon; this PR's own run is the xdist-only number. It was the slowest of the 11 (166-479 s for nearly the same tests: runner variance), so it understates the gain. Locally on 4 CPUs with coverage as in CI, the base took 825 s serial and 332-344 s with this configuration (3 runs). - The GPU, Example and Regression workflows hold their jobs until the unit `linux` check is done (`pr-gate / wait-checks`, at least 5 min). That wait is a median 14.5 min (IQR 12.9-15.8) on 125 GPU runs of 33 PRs since #2823; in this PR's own GPU run it was 9.7 min (its `linux` job took 10.9 min). With a 7 min `linux` job instead of 12.9 min, a GPU run requested right after a push starts about 6 min earlier. That figure is derived from the job times: the later runs of the series were requested after their unit check had finished (the wait sat at its 5.2 min floor in 7 of the 9 GPU runs), so they cannot show it. Method: every PR of the series has its own CI run, so "before" is the run of the PR(s) below that lack the change and "after" the runs that carry it. Times come from the job logs (`--durations`, or the gap between consecutive result lines). Where runner speed matters, the tests that no PR of the series touches scale each run, and the raw and the scaled medians are both shown. One test varies by 15-30% from run to run on its own, so smaller differences are not evidence. ### Usage Nothing user-facing. Locally: `OMP_NUM_THREADS=1 pytest tests/unit -n 4 --dist loadfile`. ### Testing - Full `tests/unit` on 4 CPUs with coverage as in CI, on the base before #2823: serial 825 s, this configuration 332-344 s in three repeated runs (4,724 passed, 0 failed). - This PR's own CI run: the `linux` job took 10.9 min (479 s of pytest) against the 12.9 min serial median; see the time section above for the later runs of the series. - `tests/unit` without `puzzletron` (it needs `hydra`, absent from my local environment) in reversed and in shuffled file order shows no other order dependence; `test_search_constraints` followed by `test_evaluate_constraints.py` in one process failed before the fix and passes after it. - `test_hf_dflash2.py` (33 tests) on one CPU thread: 13.5 s, of which `test_overfits_a_single_batch` takes about 4 s (30 s with the bf16 model on main; the next slowest test takes about 1.3 s). - Pre-commit (all hooks) passes. ### Before your PR is "*Ready for review*" Make sure you read and follow [Contributor guidelines](https://github.com/NVIDIA/Model-Optimizer/blob/main/CONTRIBUTING.md) and your commits are signed (`git commit -s -S`). Make sure you read and follow the [Security Best Practices](https://github.com/NVIDIA/Model-Optimizer/blob/main/SECURITY.md#security-coding-practices-for-contributors) (e.g. avoiding hardcoded `trust_remote_code=True`, `torch.load(..., weights_only=False)`, `pickle`, etc.). - Is this change backward compatible?: N/A (CI and tests) - If you copied code from any other sources or added a new PIP dependency, did you follow guidance in `CONTRIBUTING.md`: no new dependency (`pytest-xdist` is already in the `dev-test` extra on main) - Did you write any new necessary tests?: N/A (two existing tests fixed or sped up so they pass under xdist) - Did you update [Changelog](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: N/A (CI and tests) - Did you get Claude approval on this PR?: N/A (optional, not run) ### Additional Information Slice 1 of 11 of the GPU-CI speed-up work, split out of the reference draft #2730 (not for merge; it holds the whole series with its measurements). Merge in order; each PR stands on its own and passes CI without the later ones. 1. #2845 [OMNIML-6178] [1/11] Run the unit tests in parallel with pytest-xdist **(this PR)** 2. #2846 [OMNIML-6178] [2/11] Use the sysmon coverage core in CI 3. #2847 [OMNIML-6179] [3/11] Run manual-marked tests on nightly and on-demand GPU runs 4. #2848 [OMNIML-6181] [4/11] Give gpu_megatron's slow cold starts more time 5. #2849 [OMNIML-6179] [5/11] Make the ONNX tests cheaper: CPU-only ones to the unit lane, small models 6. #2850 [OMNIML-6179] [6/11] Use local tokenizers and calibration text in GPU tests, shrink puzzletron data 7. #2851 [OMNIML-6180] [7/11] Trim the llm_eval, llm_qat, EAGLE and megatron_bridge example tests 8. #2852 [OMNIML-6180] [8/11] Trim the DFlash regression test 9. #2853 [OMNIML-6180] [9/11] Trim the torch_onnx example lane 10. #2854 [OMNIML-6181] [10/11] Make the distributed worker pool fail fast when a worker dies 11. #2855 [OMNIML-6181] [11/11] Share one Megatron worker pool across the gpu_megatron modules <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Improved consistency and performance of automated test runs across platforms and Python configurations through controlled parallel execution. * Preserved predictable model-check ordering and isolated temporary test changes. * Increased unit-test time limits to better support process-spawning and slower runs. * Improved test reliability by explicitly controlling numeric precision and ensuring serial execution for manual CPU tests. * No end-user functionality or product behavior changed. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Shengliang Xu <shengliangx@nvidia.com>
shengliangxu
force-pushed
the
shengliangx/ci-11-megatron-session-pool
branch
from
October 20, 2026 19:43
5cd143c to
339ca31
Compare
shengliangxu
removed this pull request from stack #2859
October 21, 2026 07:05
shengliangxu
changed the base branch from
shengliangx/ci-10-pool-dead-worker
to
shengliangx/ci-9-torch-onnx-lane
October 21, 2026 07:05
shengliangxu
force-pushed
the
shengliangx/ci-11-megatron-session-pool
branch
from
October 21, 2026 07:06
339ca31 to
fbb5097
Compare
shengliangxu
added this pull request to stack #2907
October 21, 2026 07:06
shengliangxu
added a commit
that referenced
this pull request
Oct 21, 2026
### What does this PR do? Type of change: CI (test runtime) Sets `COVERAGE_CORE=sysmon` on the unit `linux` job, `gpu_tests.yml` and `regression_tests.yml`. Python 3.12 defaults to coverage's C tracer, which slows every line of Python run under it, and every lane starts coverage in each spawned worker and subprocess through `COVERAGE_PROCESS_START`, so import-heavy start-ups pay for it (the 76 s Megatron pool start, the 22 s hf_ptq processes of the unified-export tests, the puzzletron spawns, the vLLM engine core). sysmon records the same lines through `sys.monitoring`. The example runner is left to #2727, which makes the same change there. On a Python older than 3.12 coverage warns and keeps its default core, so a lane that turned out to run an older Python would be slower, not broken (the gpu and gpu_megatron logs show 3.12). ### Time improvement (measured in CI) pytest time of each lane in the run of #2845 (the base of this PR) and in this PR's run, one run each: | lane | #2845 | this PR | change | |---|---|---|---| | gpu | 2,093 s | 1,289 s | -38% | | gpu_megatron | 2,240 s | 1,212 s | -46% | | gpu_vllm v0.20.0 | 204 s | 170 s | -17% | | gpu_vllm v0.30.0 | 426 s | 328 s | -23% | | regression | 366 s | 268 s | -27% | | unit `linux` (xdist) | 479 s | 305 s | -36% | Workflow wall time: GPU tests 59.2 -> 39.6 min, Regression 21.3 -> 15.2 min. - **How much of that is this PR.** Two single runs of the same code differ by 15-30% on their own: the eight example lanes that run over 100 s, which this PR does not touch, moved by -4% to -31% (llm_sparsity +38%) between the same two runs. The deterministic part is the Megatron worker-pool start, the setup of the first test of each gpu_megatron module: 75-77 s in each of 14 modules before (all setups of 12 s or more add up to 1,365 s) and 22-27 s after (575 s), which is 790 s of the lane's 1,028 s. Locally the unit suite takes 825 s with the C tracer, 640 s with sysmon and 572 s without coverage (4 CPUs, serial). - **A `coverage.xml`-only report change was dropped from this PR.** The runs above are of its earlier head, which also had it. It showed no CI effect: the pytest summary line came 1-4 s after the last test in the #2845 runs, which still wrote the html report, and 0-3 s in this PR's runs (median 1.1 s over the 82 lane runs of the series), so the table is the sysmon core's. It would only have saved developers' local runs (about 25 s per pytest session). Method: every PR of the series has its own CI run, so "before" is the run of the PR(s) below that lack the change and "after" the runs that carry it. Times come from the job logs (`--durations`, or the gap between consecutive result lines). Where runner speed matters, the tests that no PR of the series touches scale each run, and the raw and the scaled medians are both shown. One test varies by 15-30% from run to run on its own, so smaller differences are not evidence. ### Usage Nothing user-facing. ### Testing Measured on this tree (tests/unit on 4 CPUs serial: 572 s without coverage, 825 s with the C tracer, 640 s with sysmon; `import modelopt.torch.quantization.extensions`: 17.1 s native, 23.6 s with the C tracer, 18.3 s with sysmon, with the same 362 files and 13,821 executed lines in the spawned child's data file; one TinyLlama fp8 plus TRT-LLM deploy test: 64.8 s without coverage, 103.3 s with the C tracer, 70.5 s with sysmon, identical covered lines). Pre-commit and actionlint pass. The CI runs of the series are the check on real runners (time section above). ### Before your PR is "*Ready for review*" Make sure you read and follow [Contributor guidelines](https://github.com/NVIDIA/Model-Optimizer/blob/main/CONTRIBUTING.md) and your commits are signed (`git commit -s -S`). Make sure you read and follow the [Security Best Practices](https://github.com/NVIDIA/Model-Optimizer/blob/main/SECURITY.md#security-coding-practices-for-contributors) (e.g. avoiding hardcoded `trust_remote_code=True`, `torch.load(..., weights_only=False)`, `pickle`, etc.). - Is this change backward compatible?: N/A (CI and tests) - If you copied code from any other sources or added a new PIP dependency, did you follow guidance in `CONTRIBUTING.md`: no new dependency - Did you write any new necessary tests?: N/A (CI configuration) - Did you update [Changelog](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: N/A (CI and tests) - Did you get Claude approval on this PR?: N/A (optional, not run) ### Additional Information Slice 2 of 11 of the GPU-CI speed-up work, split out of the reference draft #2730 (not for merge; it holds the whole series with its measurements). Slices 5, 8 and 10 (#2849, #2852, #2854) target main and merge independently; the other open slices form a stack and merge bottom-up. Each PR stands on its own and passes CI without the later ones. 1. #2845 [OMNIML-6178] [1/11] Run the unit tests in parallel with pytest-xdist (merged) 2. #2846 [OMNIML-6178] [2/11] Use the sysmon coverage core in CI **(this PR)** 3. #2847 [OMNIML-6179] [3/11] Run manual-marked tests on nightly and on-demand GPU runs 4. #2848 [OMNIML-6181] [4/11] Give gpu_megatron's slow cold starts more time 5. #2849 [OMNIML-6179] [5/11] Make the ONNX tests cheaper: CPU-only ones to the unit lane, small models (targets main) 6. #2850 [OMNIML-6179] [6/11] Use local tokenizers and calibration text in GPU tests, shrink puzzletron data 7. #2851 [OMNIML-6180] [7/11] Trim the llm_eval, llm_qat, EAGLE and megatron_bridge example tests 8. #2852 [OMNIML-6180] [8/11] Trim the DFlash regression test (targets main) 9. #2853 [OMNIML-6180] [9/11] Trim the torch_onnx example lane 10. #2854 [OMNIML-6181] [10/11] Make the distributed worker pool fail fast when a worker dies (targets main) 11. #2855 [OMNIML-6181] [11/11] Share one Megatron worker pool across the gpu_megatron modules <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Updated coverage collection for GPU, regression, and Linux unit-test runs to use Python’s monitoring-based coverage core on supported Python 3.12+ environments. * This configuration reduces overhead during coverage-enabled test execution, particularly for import-heavy worker processes. * No changes to application functionality or public interfaces are included. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Shengliang Xu <shengliangx@nvidia.com>
Each of the 15 gpu_megatron modules that use `dist_workers` started its own
worker pool, about 19 of the lane's 36 minutes. Fourteen logged module starts
took 75-77 s each (the GDN module 179 s: 76 s pool plus 103 s Triton JIT), plus
3.6-10 s of shutdown, and the start has a p90 of 94-98 s.
The 76 s is not distributed init (an in-process init takes 0.1 s): the spawned
child unpickles megatron_worker_teardown by reference, which imports the heavy
tests/gpu_megatron/conftest.py (transformers, megatron.core, all of modelopt and
its plugins, TE) before it reaches init_process_group. The same import chain
takes pytest itself 89 s to start here against 17 s in the gpu lane, whose
session-scoped pool, with a light teardown function, starts in 1.7 s. Module scope
was chosen when a spawn cost about 10 s ("avoids cross-test state contamination");
the git history shows no contamination incident, and the cost has grown 7.6x.
_pool_cache is now session-scoped, so the pools are keyed by world size and
shared by every module. The dist_workers fixtures stay function-scoped and get
their pool from get_live_pool, which replaces a pool that a hang killed. What
isolation the module scope gave is kept explicitly:
- megatron_worker_teardown, after every job: synchronizes CUDA first (an
asynchronous CUDA error then surfaces in the test that caused it), destroys
Megatron's model-parallel state as before, and now also the process groups it
leaves behind (about 20 NCCL groups per dispatch, each with a communicator and a
watchdog thread; test_megatron.py alone dispatches about 110 times). A pool that lives
through the whole session needs it more than a per-module pool did;
- _reset_shared_pools, an autouse module-scoped fixture: when a module starts, it
resets the process-global torch settings that helpers like set_seed change
(cudnn and TF32 flags, deterministic algorithms, RNG) to what a fresh worker
starts with (reset_worker_state, which the workers prepare by recording those
settings right after they start), in the pools that already exist. It is not part
of _get_pool, which runs once per test and would add a job to each;
- get_live_pool and the pool's abort handling: a job that times out, is interrupted
or loses a worker kills the pool and the next test gets a fresh one, for the cost
of one pool start. That also covers CUDA poisoning after the known Blackwell
illegal-memory-access flake.
Expected saving: about 17 min of the gpu_megatron job (36 min of tests to about
19), and one pool start instead of 15 means fewer chances to hit the ready
timeout.
Testing: reset_worker_state is covered by a gloo test in
tests/gpu/torch/utils/test_distributed_worker_pool.py (it runs on CPU): flags and
seed changed in a job are back at their spawn-time values afterwards. The Megatron
lane needs GPUs, so this PR's gpu_megatron job is the check. In an earlier CI run of
the whole series (with the Triton cache of another change), the lane's pytest step
took 862 s on cold caches and 336 s on warm ones, against 2,022 s for a baseline
run.
Signed-off-by: Shengliang Xu <shengliangx@nvidia.com>
A pool that lives for the whole session turns one poisoned job into many errors. In the first CI run of this PR, test_lora_then_quantize hit the known Blackwell "illegal memory access" in an NCCL broadcast. A CUDA error of that kind is sticky, so the next jobs in the same workers failed too, and because _reset_shared_pools ran a job in the poisoned pool at the start of every module, every test of every later module errored, including the ones that use no pool (3 failed, 180 errors). With a pool per module such a flake cost one module. DistributedWorkerPool.run now retires a pool after any failed job (#2836), so get_live_pool hands the next test a new one and _reset_shared_pools skips a retired pool. What is left is a reset job that itself fails or finds a dead worker: _reset_shared_pools no longer lets that error the module. run() retires the pool, and the next test that needs one gets a new pool. Testing: the Megatron lane needs GPUs, so this PR's gpu_megatron job is the check. The retirement it relies on is covered on CPU by test_a_failed_job_retires_the_pool_even_when_every_rank_replied in tests/gpu/torch/utils/test_distributed_worker_pool.py. Signed-off-by: Shengliang Xu <shengliangx@nvidia.com>
…ron teardown The first two CI runs of this PR each hit "an illegal memory access was encountered" in test_megatron_peft (test_lora_then_quantize, then test_forward_with_one_lora[lora_config1]), after about 230 jobs in the shared worker. The lane almost never fails that way on other PRs, and it passed in the three runs of the draft this change was split from, whose teardown collected garbage right after destroying the groups; the version in this PR did not. The cause is not established. The groups are destroyed explicitly, but the finished test's Megatron and TE modules can still reference them, and without a collection their finalizers run at an arbitrary point of the next job, while its NCCL kernels may be in flight. Collecting right after the groups are destroyed, with nothing running, is the safe place for that. The cost is one collection per job, about 0.1-0.3 s. Testing: the Megatron lane needs GPUs, so this PR's gpu_megatron job is the check. The failure was intermittent, so the job has to pass repeatedly: it is re-run on this commit. Signed-off-by: Shengliang Xu <shengliangx@nvidia.com>
shengliangxu
force-pushed
the
shengliangx/ci-11-megatron-session-pool
branch
from
October 21, 2026 07:57
fbb5097 to
cf0d7de
Compare
shengliangxu
added a commit
that referenced
this pull request
Oct 21, 2026
### What does this PR do? Type of change: CI (test runtime, tests only) **The DFlash regression trains 500 steps and resumes for 20.** `test_dflash_training` trained 3 epochs (1,500 steps) and `test_dflash_resume` then a fourth epoch (500 more). On the 1-GPU PR runner the recorded durations solve to about 33 s of start-up plus 55-73 ms per step, so the two tests spend roughly 115-140 s and 60-70 s. The loss assertion only needs a final loss below 3.0, and the L40 baseline is already 1.78 at step 500. The resume still goes through the whole restore-and-train path, and it now reads `trainer_state.json` and checks that `global_step` is 520 and that a log entry follows step 500 (it had no assertion besides the exit status). The step count is fixed with `max_steps` rather than `num_train_epochs`, because the regression job runs on 2 GPUs off pull requests, where `accelerate launch` starts one process per visible GPU and an epoch is 250 steps. Measured: 81 s per PR regression job. Risk: the 1.78 baseline comes from a 1,500-step run, and with `max_steps=500` the linear schedule decays to zero at step 500, so the final loss can differ from it; the bound leaves 1.2 of margin. ### Time improvement (measured in CI) | lane | | before | after | change | |---|---|---|---|---| | regression | `test_dflash_training` | 118 s | 62 s | -56 s | | regression | `test_dflash_resume` | 56 s | 31 s | -25 s | | regression | **lane pytest** | 268 s | 170 s | **-98 s (-37%)** | DFlash: before = #2846, #2848 and #2850 (#2849 ran on a slower runner), after = #2852, #2854 and #2855 (median; the lane took 168 and 170 s on the normal-speed runners and 243 s on the slow one). On its own, on main (2026-10-21): this PR's regression job against #2854's, which has the same base and lacks this change. The unchanged DFlash tests took the same time within about 3% (equally fast runners). `test_dflash_training` takes 144 s before and 83 s after, `test_dflash_resume` 73 s and 47 s, and the lane's pytest time goes from 374 s to 291 s (-83 s). The lane passes (5 passed), so the loss stays below 3.0 after 500 steps and the resume ends at `global_step` 520. A vLLM change that sized the KV pool of the Qwen3-MoE and DeepSeek engines down was dropped from this PR. The runs above are of its earlier head, which had it: the engines it touched booted 5-6 s per vLLM version faster than the ones it left alone, in line with the 1-3 s per boot expected and below the boot-to-boot noise (the unchanged Llama engine took 84-99 s on v0.30.0), so it saved nothing measurable. Method: every PR of the series has its own CI run, so "before" is the run of the PR(s) below that lack the change and "after" the runs that carry it. Times come from the job logs (`--durations`, or the gap between consecutive result lines). Where runner speed matters, the tests that no PR of the series touches scale each run, and the raw and the scaled medians are both shown. One test varies by 15-30% from run to run on its own, so smaller differences are not evidence. ### Usage Nothing user-facing. ### Testing I read the transformers 4.57.1 sources (`max_steps` overrides `num_train_epochs`; with the steps save strategy a checkpoint is saved at step 500 and again at the end) and the accelerate 1.10.0 launcher defaults. Not run on a GPU here: this PR's regression job is the check, in particular that the final DFlash loss stays below 3.0 after 500 steps and that the resume ends at `global_step` 520. Pre-commit passes. ### Before your PR is "*Ready for review*" Make sure you read and follow [Contributor guidelines](https://github.com/NVIDIA/Model-Optimizer/blob/main/CONTRIBUTING.md) and your commits are signed (`git commit -s -S`). Make sure you read and follow the [Security Best Practices](https://github.com/NVIDIA/Model-Optimizer/blob/main/SECURITY.md#security-coding-practices-for-contributors) (e.g. avoiding hardcoded `trust_remote_code=True`, `torch.load(..., weights_only=False)`, `pickle`, etc.). - Is this change backward compatible?: N/A (CI and tests) - If you copied code from any other sources or added a new PIP dependency, did you follow guidance in `CONTRIBUTING.md`: no new dependency - Did you write any new necessary tests?: N/A (existing tests made cheaper; the DFlash resume test now also asserts on its result) - Did you update [Changelog](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: N/A (CI and tests) - Did you get Claude approval on this PR?: N/A (optional, not run) ### Additional Information Slice 8 of 11 of the GPU-CI speed-up work, split out of the reference draft #2730 (not for merge; it holds the whole series with its measurements). Slices 5, 8 and 10 (#2849, #2852, #2854) target main and merge independently; the other open slices form a stack and merge bottom-up. Each PR stands on its own and passes CI without the later ones. The times above are from the series' CI runs before 2026-10-21, when all slices formed one linear stack; a PR number in a method note names that PR's run from then. 1. #2845 [OMNIML-6178] [1/11] Run the unit tests in parallel with pytest-xdist (merged) 2. #2846 [OMNIML-6178] [2/11] Use the sysmon coverage core in CI 3. #2847 [OMNIML-6179] [3/11] Run manual-marked tests on nightly and on-demand GPU runs 4. #2848 [OMNIML-6181] [4/11] Give gpu_megatron's slow cold starts more time 5. #2849 [OMNIML-6179] [5/11] Make the ONNX tests cheaper: CPU-only ones to the unit lane, small models (targets main) 6. #2850 [OMNIML-6179] [6/11] Use local tokenizers and calibration text in GPU tests, shrink puzzletron data 7. #2851 [OMNIML-6180] [7/11] Trim the llm_eval, llm_qat, EAGLE and megatron_bridge example tests 8. #2852 [OMNIML-6180] [8/11] Trim the DFlash regression test (targets main) **(this PR)** 9. #2853 [OMNIML-6180] [9/11] Trim the torch_onnx example lane 10. #2854 [OMNIML-6181] [10/11] Make the distributed worker pool fail fast when a worker dies (targets main) 11. #2855 [OMNIML-6181] [11/11] Share one Megatron worker pool across the gpu_megatron modules <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Updated regression coverage to use fixed training limits of 500 steps, with resumed runs extending to step 520. * Improved checkpoint and resume validation to confirm the final training step and expected post-resume progress logs. * Updated loss expectations to reflect the result at the 500-step training milestone. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Shengliang Xu <shengliangx@nvidia.com>
shengliangxu
added a commit
that referenced
this pull request
Oct 21, 2026
… a worker dies (#2854) ### What does this PR do? Type of change: CI (test reliability); changes `tests/_test_utils` only A worker that exits without reporting (an illegal memory access aborts the process, or an OOM kill) was only noticed when the result timeout ran out: 600 s for a job, 120 s for the start. Until then the parent waits on a queue nobody writes to, and the other workers wait in the job, typically in a collective, for their dead peer. A start that fails, for example on a bad backend, also waited out the whole ready timeout and left the workers it had spawned behind. `DistributedWorkerPool` now checks the workers' liveness once a second while it waits. A dead worker is noticed within a second and fails the run, or the start, naming its rank and exit code (about 4 s after the death, which includes the 3 s the pool already waits for the other workers' stack dumps). If another rank had already reported an exception, its traceback is kept in that error. The pool is marked `broken` and the other workers are killed with their stacks printed, the way a hung job or a failed rank already ends in main (`get_live_pool` then hands out a new pool). A start that fails kills the workers it had spawned. `ready_timeout` is a constructor argument (120 s by default, as before), so a test can reach the start timeout without waiting for it. This builds on the hang handling of #2832 (`broken`, `get_live_pool`, the stack dump) and does not change it. ### Time improvement (measured locally, not in CI) No lane time changes in a passing run, and no worker died in this series' CI runs, so CI cannot show it. What changes is how long a dead worker costs. CPU, gloo, two ranks, the pool test's own job (rank 0 exits with code 13, rank 1 keeps waiting), seconds from `pool.run(...)` (or the constructor) to the exception, on main and on this PR: | scenario | before | after | |---|---|---| | a rank dies mid-job, its peer is stuck in a wait, `job_timeout` 30 s (as in the test) | 33.1 s, `queue.Empty` | 4.0 s, `Worker process(es) died (rank, exit code): [(0, 13)]` | | the same with the default `job_timeout` of 600 s | 603.1 s, `queue.Empty` | 4.0 s | | the same, the peer waits in `dist.barrier()` | 33.3 s (the 30 s `failure_grace`) | 4.3 s | | the start fails (`backend="not-a-backend"`), default `ready_timeout` of 120 s | 120.1 s, `queue.Empty` | 5.0 s | | rank 0 dies at the start, rank 1 hangs in the rendezvous | 120.0 s, one live worker left behind | 5.0 s, no worker left | The after time is the 1 s liveness poll plus the 3 s the pool waits for the other workers' stack dumps before it kills them. At the production timeouts (120 s for the start, 600 s for a job) a dead worker therefore costs 4-5 s instead of 2-10 minutes of a CI job: the test fails at once and the next one gets a fresh pool from `get_live_pool`, instead of waiting for a queue nobody writes to. Cost: the four new cases add about 27 s to the `gpu` lane: the pool test module takes 43 s in this PR's run and 17 s in #2849's run, which has the same base (main) and lacks them (both from the job logs). Method: a small script (not part of this PR) builds the pool the way the repo's test does and times each scenario with `time.monotonic()` around the call, once per commit, on a CPU host. ### Usage ```python pool = DistributedWorkerPool(world_size=2, backend="nccl", ready_timeout=300) # new argument, default 120 ``` ### Testing `tests/gpu/torch/utils/test_distributed_worker_pool.py` runs the pool with gloo on CPU (it extends main's three tests; the new cases take 24 s together): - a rank that exits with code 13 while its peer waits fails the run in about 5 s and kills the peer. The test sets `job_timeout = 30`, so a regression fails there with `queue.Empty` instead of at the test's own cap. With the liveness check disabled it fails exactly that way. - the same test with a second job: rank 0 raises an exception and rank 1 exits with code 13 two seconds later, after rank 0 has reported. The error carries the exit code and rank 0's traceback. Without the change it carries only the exit code, and the case fails. - a start that fails raises and leaves no worker, both when every worker dies (a bad backend) and when the wait times out while they are still starting (`ready_timeout=0.01`). With `_abort()` removed from the failed-start path, the second case fails ("workers left behind"); the first passes either way, since all its workers die on their own, so it covers the liveness wiring only. Main's three tests (as reworked by #2836) still pass, and the three new cases fail against main's unmodified pool. Pre-commit passes. This PR is rebased onto #2836, which also edited the pool (reporting a failed rank before its teardown, retiring a pool after any failed job); the changes are in different places. The default start timeout stays 120 s here; #2848 raises it to 300 s, so whichever of the two merges second resolves that one line. ### Before your PR is "*Ready for review*" Make sure you read and follow [Contributor guidelines](https://github.com/NVIDIA/Model-Optimizer/blob/main/CONTRIBUTING.md) and your commits are signed (`git commit -s -S`). Make sure you read and follow the [Security Best Practices](https://github.com/NVIDIA/Model-Optimizer/blob/main/SECURITY.md#security-coding-practices-for-contributors) (e.g. avoiding hardcoded `trust_remote_code=True`, `torch.load(..., weights_only=False)`, `pickle`, etc.). - Is this change backward compatible?: N/A (CI and tests) - If you copied code from any other sources or added a new PIP dependency, did you follow guidance in `CONTRIBUTING.md`: no new dependency - Did you write any new necessary tests?: ✅ 2 test functions (4 cases) in `tests/gpu/torch/utils/test_distributed_worker_pool.py`, run on CPU with gloo; each fails without its fix - Did you update [Changelog](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: N/A (CI and tests) - Did you get Claude approval on this PR?: N/A (optional, not run) ### Additional Information Slice 10 of 11 of the GPU-CI speed-up work, split out of the reference draft #2730 (not for merge; it holds the whole series with its measurements). Slices 5, 8 and 10 (#2849, #2852, #2854) target main and merge independently; the other open slices form a stack and merge bottom-up. Each PR stands on its own and passes CI without the later ones. The times above are from the series' CI runs before 2026-10-21, when all slices formed one linear stack; a PR number in a method note names that PR's run from then. 1. #2845 [OMNIML-6178] [1/11] Run the unit tests in parallel with pytest-xdist (merged) 2. #2846 [OMNIML-6178] [2/11] Use the sysmon coverage core in CI 3. #2847 [OMNIML-6179] [3/11] Run manual-marked tests on nightly and on-demand GPU runs 4. #2848 [OMNIML-6181] [4/11] Give gpu_megatron's slow cold starts more time 5. #2849 [OMNIML-6179] [5/11] Make the ONNX tests cheaper: CPU-only ones to the unit lane, small models (targets main) 6. #2850 [OMNIML-6179] [6/11] Use local tokenizers and calibration text in GPU tests, shrink puzzletron data 7. #2851 [OMNIML-6180] [7/11] Trim the llm_eval, llm_qat, EAGLE and megatron_bridge example tests 8. #2852 [OMNIML-6180] [8/11] Trim the DFlash regression test (targets main) 9. #2853 [OMNIML-6180] [9/11] Trim the torch_onnx example lane 10. #2854 [OMNIML-6181] [10/11] Make the distributed worker pool fail fast when a worker dies (targets main) **(this PR)** 11. #2855 [OMNIML-6181] [11/11] Share one Megatron worker pool across the gpu_megatron modules <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Distributed worker pools now detect workers that exit during startup or job execution and report failures instead of waiting for the full job timeout. * Worker startup supports a configurable readiness timeout with prompt timeout handling. * Failed or timed-out initialization now cleans up spawned worker processes, preventing orphaned processes and leaving the pool in a consistent state. * Job result retrieval now responds promptly when a worker fails. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Shengliang Xu <shengliangx@nvidia.com>
This branch has not been deployed
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
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 does this PR do?
Type of change: CI (test runtime); changes
tests/gpu_megatronandtests/_test_utilsonlyEach of the 15 gpu_megatron modules that use
dist_workersstarted its own worker pool: about 19 of the lane's 36 minutes. Fourteen logged module starts took 75-77 s each (the GDN module 179 s: 76 s pool plus 103 s Triton JIT), plus 3.6-10 s of shutdown, and the start has a p90 of 94-98 s.The 76 s is not distributed init (an in-process init takes 0.1 s): the spawned child unpickles
megatron_worker_teardownby reference, which imports the heavytests/gpu_megatron/conftest.py(transformers, megatron.core, all of modelopt and its plugins, TE) before it reachesinit_process_group. The same import chain takes pytest itself 89 s to start here against 17 s in the gpu lane, whose session-scoped pool, with a light teardown function, starts in 1.7 s. Module scope was chosen when a spawn cost about 10 s ("avoids cross-test state contamination"); the git history shows no contamination incident, and the cost has grown 7.6x._pool_cacheis now session-scoped, so the pools are keyed by world size and shared by every module. Thedist_workersfixtures stay function-scoped and get their pool fromget_live_pool, which replaces a pool that a hang killed. What isolation the module scope gave is kept explicitly:megatron_worker_teardown, after every job: synchronizes CUDA first (an asynchronous CUDA error then surfaces in the test that caused it), destroys Megatron's model-parallel state as before, and now also the process groups it leaves behind (about 20 NCCL groups per dispatch, each with a communicator and a watchdog thread;test_megatron.pyalone dispatches about 110 times). A pool that lives through the whole session needs it more than a per-module pool did._reset_shared_pools, an autouse module-scoped fixture: when a module starts, it resets the process-global torch settings that helpers likeset_seedchange (cudnn and TF32 flags, deterministic algorithms, RNG) to what a fresh worker starts with (reset_worker_state; the workers record those settings right after they start), in the pools that already exist. It is not part of_get_pool, which runs once per test and would add a job to each.get_live_pooland the pool's abort handling: a job that times out, is interrupted, loses a worker or fails on any rank retires the pool, and the next test gets a fresh one, for the cost of one pool start.DistributedWorkerPool.runhas retired a pool after a plain failed job since CI follow-up: cache source-built wheels and extensions, bump CI containers, trim slow GPU tests #2836, which matters more with a session-wide pool: a CUDA error such as the known Blackwell illegal memory access is sticky, the workers stay poisoned, and every later test would fail with it._reset_shared_poolsno longer lets a failed reset error the module's tests (they may not use a pool at all).Measured saving on top of #2846's sysmon core: 216 s of the gpu_megatron lane (1,038 s to 822 s; see the time section). Without sysmon a pool start costs 76 s, and the saving would be about 17 min (an estimate; the series was not run that way). One pool start instead of 15 also means fewer chances to hit the ready timeout.
Time improvement (measured in CI)
gpu_megatron lane, pytest time:
This PR on its own: 1,038 s -> 822 s, -216 s (-21%), against the two earlier runs (#2852: 1,029 s, #2854: 1,038 s). This PR's run was on a slightly slower runner (the heavy
callphases of the tests it does not change took 6% longer than in #2854), so the gain is not runner luck. The setups of 12 s or more, which are the pool starts and the module-level compiles, fall from 16 (489 s) to 3 (246 s): the pool starts once instead of in each of 14 modules, and what remains is the extension build (124 s), the GDN module's Triton JIT (105 s) and one 18 s pool start.With sysmon (#2846) a pool start takes 18-27 s instead of 75-77 s, so each avoided start saves less than the 76 s this PR was designed around. Without #2846 the saving would be 14 starts of 76 s plus the 3.6-10 s shutdowns, about 17 min; that is an estimate, because the series was never run without sysmon on this PR.
This is one CI run of the final head: the two runs before it failed on the sticky CUDA error and on the missing
gc.collect()described below.Method: every PR of the series has its own CI run, so "before" is the run of the PR(s) below that lack the change and "after" the runs that carry it. Times come from the job logs (
--durations, or the gap between consecutive result lines). Where runner speed matters, the tests that no PR of the series touches scale each run, and the raw and the scaled medians are both shown. One test varies by 15-30% from run to run on its own, so smaller differences are not evidence.Usage
Nothing user-facing (test fixtures).
Testing
reset_worker_statehas a gloo test next to the other pool tests (it runs on CPU): flags and seed changed in a job are back at their spawn-time values afterwards; it fails when the helper forgets one of the settings.test_lora_then_quantizeand poisoned the shared pool, and every later module errored (3 failed, 180 errors), because the pool was reused after a failed job and_reset_shared_poolsran a job in it at each module start. The second commit retired a pool after any failed job and made a failed reset harmless; with it the lane ended with 1 failure instead of 183, intest_forward_with_one_lora[lora_config1](the same CUDA error). Re-running the job on that commit failed at the same test again, so it was not a random flake: the teardown of this PR, unlike the draft's, did not collect garbage after destroying the groups, and the draft passed the lane three times. The third commit adds thatgc.collect(); the lane then passes. The cause is not established (the finished test's Megatron and TE modules can still reference the destroyed groups, and their finalizers then run at an arbitrary point of the next job). CI follow-up: cache source-built wheels and extensions, bump CI containers, trim slow GPU tests #2836 has since landed the retirement inrun()for its own reasons (with its own gloo test), so the second commit now only makes a failed reset harmless; main's teardown has neither the process-group loop nor the collection, and a session-wide pool needs both.Before your PR is "Ready for review"
Make sure you read and follow Contributor guidelines and your commits are signed (
git commit -s -S).Make sure you read and follow the Security Best Practices (e.g. avoiding hardcoded
trust_remote_code=True,torch.load(..., weights_only=False),pickle, etc.).CONTRIBUTING.md: no new dependencytests/gpu/torch/utils/test_distributed_worker_pool.py(gloo, CPU) for the new helper; the session pool itself is exercised by the whole gpu_megatron laneAdditional Information
Slice 11 of 11 of the GPU-CI speed-up work, split out of the reference draft #2730 (not for merge; it holds the whole series with its measurements). Slices 1, 2, 8 and 10 (#2845, #2846, #2852, #2854) have merged. Slice 5 (#2849) targets main and merges independently; the other open slices form a stack and merge bottom-up. Each PR stands on its own and passes CI without the later ones. The times above are from the series' CI runs before 2026-10-21, when all slices formed one linear stack; a PR number in a method note names that PR's run from then.
Summary by CodeRabbit