From 411d072a2e7caad083ee858de8a20caf8b2043f0 Mon Sep 17 00:00:00 2001 From: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com> Date: Wed, 2 Sep 2026 22:08:56 +0530 Subject: [PATCH] Speed up megatron_bridge example tests by ~6x on a single GPU (#2296) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### What does this PR do? Type of change: Test infrastructure / CI time `tests/examples/megatron_bridge` spends most of its time importing Python, not testing. Each step of a test spawns `torchrun`, and the new process spends **~25s importing torch/megatron/modelopt** before doing any work. A single `test_qad` run pays that **six** times — three steps, plus a spawned child per distributed checkpoint save, because Megatron-Core's async writer uses `mp_mode="spawn"` and spawn re-imports `__main__`. Profiled with phase timers in the example scripts: | | share of `test_qad[qwen3]` | |---|---| | Python imports (6 process launches × ~26s) | **~76%** | | actual compute (`mtq.quantize` 4.2s, model build 0.24s, export 0.06s) | ~5s | `run_example_command` now dispatches each step internally instead of shelling out: - **single-rank steps** run directly in the pytest process, driving the script's own `get_args()` + `main()` — no new interpreter, no re-import; - **multi-rank steps** drive `torch.distributed.run.main()` in-process with patched `sys.argv`, the same pattern Megatron-Bridge uses in its own functional tests. ### Results | suite | before | after | |---|---|---| | `tests/examples/megatron_bridge`, **1 GPU** | **21m27** | **4m03 - 6m17** | | `tests/examples/megatron_bridge`, **2 GPU** | 26m10 | 25m03 | Both figures are on current `main` (17 tests). The 2-GPU number is up from 20m41 before merging #2276, which added a test and made one previously single-rank step multi-rank. The 1-GPU figure is a range, not a best case. Across ten runs on a verified-idle box the suite lands at ~4m most of the time and at ~6m otherwise, always with the same result (17 passed). Per-test durations show the entire spread is one test: `test_qad[qwen3]` runs at ~15s or at ~149s. It is 25s in isolation, 15s after `test_distill.py` and ~25s after `test_prune_minitron.py`, so it needs the full sequence and does not reproduce on demand — three attempts to catch it under instrumentation all landed on fast runs. In those, CUDA state immediately before it is 46 MiB allocated / 68 MiB reserved / 7 segments / 22 MiB inactive-split, and the preceding test's 498/984 MiB is fully reclaimed, so a fragmented allocator is measured *not* to be the cause in the fast path at least. Left documented rather than guessed at: correctness is unaffected across every run, and the worst case sits inside the 30-minute PR budget (CI `Run tests` 902s). The single-GPU path is the big win, and it is the one the per-PR runner uses — that job now finishes in **8 minutes** in CI. Multi-rank steps still launch worker processes that re-import, so the 2-GPU nightly improves far less. This also fixes the timeouts under coverage. With `--cov` (how CI runs it), on the same three tests: in-process **3 passed in 1m15**, subprocess **3 failed on `Timeout (>360.0s)` in 18m57**. ### CI timeout The 2-GPU nightly runs every test multi-GPU and measured **58 minutes against a 60-minute cap** — too close to be reliable. `timeout_minutes` is now ref-conditional, mirroring the `runner` line directly below it: **30 minutes on PRs** (single-GPU, ~8 min) and **75 on nightly**. Keeping the nightly at full multi-GPU coverage is deliberate. Making individual tests single-rank cut it to ~7 minutes, but it gives up the parallel-path coverage that is the whole point of the 2-GPU job, and it surfaced a real fragility: `test_prune_minitron[nemotron_h]` fails with *"No scores collected for importance estimation"* when it runs single-rank after the full distill file. It passes alone and after any single preceding test — multi-rank tests are immune because `torchrun` gives them fresh worker processes. Nightly is the right place to spend the wall-clock. ### What is and isn't covered Each script's real `get_args()` still runs, so CLI flags, defaults and recipe-string resolution stay covered. Not covered for single-rank steps: the `torchrun` invocation itself and the `__main__` block (`dist.setup()` / `dist.abort()`). Multi-rank steps still go through the real launcher. **No test file changes.** The tests still read as "launch this torchrun command" and their assertions are untouched. ### Keeping it that way There is no toggle and no fallback. Every step in this suite must be `torchrun --nproc_per_node=