.github/workflows/test.yml runs distributed-tests only on push to main, and e2e-tests only on workflow_dispatch. Neither runs on a pull request. The result is that a PR can be fully green and still break multi-GPU code, with the failure surfacing only after merge — in a job whose long per-file log nobody reads.
This is how TestKVCacheTP2 stayed broken for three months (#88), and it is why #86 could change TP-related code — the tensor-parallel RNG tracker, weight init — with no multi-GPU verification anywhere in its review.
Compounding it: test_13b_offload_capability.py and the two GRPO-DDP tests in test_reward_manager.py::TestRLVRTraining are real, correct tests that no automatic trigger ever runs, since e2e is dispatch-only.
Options worth weighing:
- Run
distributed-tests on PRs too. Costs 2-GPU runner time on every PR; node1 is often occupied, which is what motivated registering halo.
- Keep PRs as they are, add a nightly or pre-merge scheduled run on main with a failure notification, so a break is caught in hours rather than at the next person's PR.
- Run distributed tests on PRs only when the diff touches
ironcore/parallel/, ironcore/offload/, or tests/multi_gpu/.
(3) is probably the best cost/coverage trade, but it needs someone to own the path list. Flagging rather than picking, since it is a repo policy call.
Related capability note: these cannot be run on the halo runner. RCCL refuses two ranks on one device — Duplicate GPU detected : rank 0 and rank 1 both on CUDA device (NCCL 2.27.7) — so a 2-GPU host is required regardless of which option is chosen.
.github/workflows/test.ymlrunsdistributed-testsonly on push to main, ande2e-testsonly onworkflow_dispatch. Neither runs on a pull request. The result is that a PR can be fully green and still break multi-GPU code, with the failure surfacing only after merge — in a job whose long per-file log nobody reads.This is how
TestKVCacheTP2stayed broken for three months (#88), and it is why #86 could change TP-related code — the tensor-parallel RNG tracker, weight init — with no multi-GPU verification anywhere in its review.Compounding it:
test_13b_offload_capability.pyand the two GRPO-DDP tests intest_reward_manager.py::TestRLVRTrainingare real, correct tests that no automatic trigger ever runs, sincee2eis dispatch-only.Options worth weighing:
distributed-testson PRs too. Costs 2-GPU runner time on every PR;node1is often occupied, which is what motivated registeringhalo.ironcore/parallel/,ironcore/offload/, ortests/multi_gpu/.(3) is probably the best cost/coverage trade, but it needs someone to own the path list. Flagging rather than picking, since it is a repo policy call.
Related capability note: these cannot be run on the
halorunner. RCCL refuses two ranks on one device —Duplicate GPU detected : rank 0 and rank 1 both on CUDA device(NCCL 2.27.7) — so a 2-GPU host is required regardless of which option is chosen.