Skip to content

Commit 839d8c5

Browse files
TianyeGGBondclaude
andcommitted
refactor(dual-driver): PipelineHandle dataclass + local imports
_build_pipeline took six threaded dependencies (ray, MilesCoordinator, MilesPipelineConfig, get_coordinator_actor_name, get_pipeline_namespace, logger) and returned a 5-tuple that main() then re-packed into a 7-tuple, unpacked positionally in four places. A missed position would silently mis-wire a handle. - Drop the threaded deps: _build_pipeline imports ray / MilesCoordinator / get_coordinator_actor_name / get_pipeline_namespace locally (still after the env guard, since it only runs from main); MilesPipelineConfig and the logger are module-level. - Return a PipelineHandle dataclass; main fills train_group / rollout_manager after build and accesses everything by attribute. No behavior change; both drivers still import cleanly with the heavy imports deferred past the RLIX_CONTROL_PLANE guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 131181c commit 839d8c5

1 file changed

Lines changed: 60 additions & 57 deletions

File tree

examples/rlix/run_miles_dual.py

Lines changed: 60 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@
3737
from __future__ import annotations
3838

3939
import copy
40+
import logging
4041
import os
42+
from dataclasses import dataclass
43+
from typing import Any
4144

4245
from ._common import (
4346
MilesPipelineConfig,
@@ -51,6 +54,22 @@
5154
"examples/rlix/run_miles_dual.py", "examples.rlix.run_miles_dual"
5255
)
5356

57+
logger = logging.getLogger("run_miles_dual")
58+
59+
60+
@dataclass
61+
class PipelineHandle:
62+
"""One pipeline's actors + handles, populated across build and setup."""
63+
64+
index: int
65+
pipeline_id: str
66+
namespace: str
67+
coordinator: Any
68+
pipeline: Any
69+
args: Any
70+
train_group: Any = None
71+
rollout_manager: Any = None
72+
5473

5574
def _split_pools_for_dual(
5675
*, num_gpus_per_node: int, infer_pool_size: int
@@ -217,13 +236,7 @@ def _build_pipeline(
217236
train_mapping: list[int],
218237
infer_mapping: list[int],
219238
orchestrator,
220-
ray,
221-
MilesCoordinator,
222-
MilesPipelineConfig,
223-
get_coordinator_actor_name,
224-
get_pipeline_namespace,
225-
logger,
226-
):
239+
) -> PipelineHandle:
227240
"""Allocate one pipeline_id, register, admit, create coordinator+pipeline.
228241
229242
``train_mapping`` / ``infer_mapping`` are the EXPLICIT physical GPU
@@ -232,8 +245,15 @@ def _build_pipeline(
232245
partial-overlap invariant is asserted; cross-pipeline overlap is
233246
asserted by ``grep_overlap_log.sh`` end-to-end.
234247
235-
Returns ``(pipeline_id, namespace, coordinator_handle, pipeline_handle, args)``.
248+
Returns a :class:`PipelineHandle`.
236249
"""
250+
import ray
251+
from rlix.pipeline.miles_coordinator import MilesCoordinator
252+
from rlix.protocol.types import (
253+
get_coordinator_actor_name,
254+
get_pipeline_namespace,
255+
)
256+
237257
pipeline_id = ray.get(orchestrator.allocate_pipeline_id.remote("miles"))
238258
pipeline_namespace = get_pipeline_namespace(pipeline_id)
239259

@@ -331,32 +351,32 @@ def _build_pipeline(
331351
pipeline_index, pipeline_id,
332352
)
333353

334-
return pipeline_id, pipeline_namespace, coordinator, pipeline, args
354+
return PipelineHandle(
355+
index=pipeline_index,
356+
pipeline_id=pipeline_id,
357+
namespace=pipeline_namespace,
358+
coordinator=coordinator,
359+
pipeline=pipeline,
360+
args=args,
361+
)
335362

336363

337364
def main():
338365
"""Dual-pipeline entry. Imports heavy modules lazily so the env-var
339366
guard above fires before transitive ``import torch`` / ``import sglang``.
340367
"""
341368
import asyncio
342-
import logging
343369

344370
import ray
345371

346372
from miles.utils.arguments import parse_args
347373
from miles.utils.logging_utils import configure_logger
348374
from miles.utils.rlix_train_loop import run_async_train_loop
349375
from miles.utils.rlix_validation import assert_rlix_topology
350-
from rlix.pipeline.miles_coordinator import MilesCoordinator
351-
from rlix.protocol.types import (
352-
get_coordinator_actor_name,
353-
get_pipeline_namespace,
354-
)
355376

356377
import rlix
357378

358379
configure_logger()
359-
logger = logging.getLogger("run_miles_dual")
360380
base_args = parse_args()
361381

362382
# F10 startup fail-fast on the BASE args. Per-pipeline arg overrides
@@ -411,75 +431,64 @@ def main():
411431
train_mapping=p1_train,
412432
infer_mapping=p1_infer,
413433
orchestrator=orchestrator,
414-
ray=ray,
415-
MilesCoordinator=MilesCoordinator,
416-
MilesPipelineConfig=MilesPipelineConfig,
417-
get_coordinator_actor_name=get_coordinator_actor_name,
418-
get_pipeline_namespace=get_pipeline_namespace,
419-
logger=logger,
420434
)
421435
p2 = _build_pipeline(
422436
base_args=base_args,
423437
pipeline_index=2,
424438
train_mapping=p2_train,
425439
infer_mapping=p2_infer,
426440
orchestrator=orchestrator,
427-
ray=ray,
428-
MilesCoordinator=MilesCoordinator,
429-
MilesPipelineConfig=MilesPipelineConfig,
430-
get_coordinator_actor_name=get_coordinator_actor_name,
431-
get_pipeline_namespace=get_pipeline_namespace,
432-
logger=logger,
433441
)
434442

435-
pipelines = [p1, p2]
443+
handles = [p1, p2]
436444

437445
# ---- 3. Pull handles for each pipeline. ------------------------------
438-
handles = []
439-
for pid, ns, coord, pipe, args in pipelines:
440-
train_group = ray.get(pipe.get_train_group.remote())
441-
rollout_manager = ray.get(pipe.get_rollout_manager.remote())
442-
engine_count = int(ray.get(pipe.get_declared_engine_count.remote()))
446+
for h in handles:
447+
h.train_group = ray.get(h.pipeline.get_train_group.remote())
448+
h.rollout_manager = ray.get(h.pipeline.get_rollout_manager.remote())
449+
engine_count = int(ray.get(h.pipeline.get_declared_engine_count.remote()))
443450
logger.info(
444451
"[run_miles_dual] handles ready pipeline_id=%s engines=%d",
445-
pid, engine_count,
452+
h.pipeline_id, engine_count,
446453
)
447-
handles.append((pid, ns, coord, pipe, args, train_group, rollout_manager))
448454

449455
# ---- 4. Drive 2 concurrent rlix_train_loops via asyncio.gather. -----
450-
async def _run_one_pipeline(idx, pid, pipe, args, train_group, rollout_manager):
456+
async def _run_one_pipeline(h: PipelineHandle):
451457
async def _before(step: int) -> None:
452-
await pipe.before_training.remote(step)
458+
await h.pipeline.before_training.remote(step)
453459

454460
async def _after(step: int) -> None:
455-
await pipe.after_training.remote(step)
461+
await h.pipeline.after_training.remote(step)
456462

457463
async def _release_only(step: int) -> None:
458-
# R04-F1 cleanup hook: releases actor_train allocation only.
459-
await pipe.release_train_only.remote(step)
464+
# Cleanup hook: releases actor_train allocation only.
465+
await h.pipeline.release_train_only.remote(step)
460466

461467
# Per-rollout step_target = rollout_batch_size. See
462468
# MilesPipeline.signal_rollout_demand docstring for why pre-signalling
463469
# demand to the scheduler is required for 4-GPU 2-pipeline full
464470
# cross-overlap (without it, rollout 2+ hangs when both pipelines
465471
# release all DP workers between rollouts).
466-
_step_target = int(getattr(args, "rollout_batch_size", 0) or 0)
472+
_step_target = int(getattr(h.args, "rollout_batch_size", 0) or 0)
467473

468474
async def _signal_demand(rollout_id: int) -> None:
469475
if _step_target <= 0:
470476
return
471-
await pipe.signal_rollout_demand.remote(rollout_id, _step_target)
477+
await h.pipeline.signal_rollout_demand.remote(rollout_id, _step_target)
472478

473479
await run_async_train_loop(
474-
args,
475-
train_group=train_group,
476-
rollout_manager=rollout_manager,
480+
h.args,
481+
train_group=h.train_group,
482+
rollout_manager=h.rollout_manager,
477483
before_step=_before,
478484
after_step=_after,
479485
release_only=_release_only,
480486
signal_demand=_signal_demand,
481487
)
482-
logger.info("[run_miles_dual] mp%d training loop complete pipeline_id=%s", idx, pid)
488+
logger.info(
489+
"[run_miles_dual] mp%d training loop complete pipeline_id=%s",
490+
h.index, h.pipeline_id,
491+
)
483492

484493
async def _async_main():
485494
# F4 fix (m11-review.review-report.md §2): use create_task + wait(
@@ -491,13 +500,7 @@ async def _async_main():
491500
# so Phase 1's try/finally inside run_async_train_loop fires
492501
# release_only on the CancelledError path and the scheduler
493502
# ledger stays consistent.
494-
tasks = [
495-
asyncio.create_task(
496-
_run_one_pipeline(i + 1, pid, pipe, args, train_group, rollout_manager)
497-
)
498-
for i, (pid, ns, coord, pipe, args, train_group, rollout_manager)
499-
in enumerate(handles)
500-
]
503+
tasks = [asyncio.create_task(_run_one_pipeline(h)) for h in handles]
501504
try:
502505
done, pending = await asyncio.wait(
503506
tasks, return_when=asyncio.FIRST_EXCEPTION
@@ -530,13 +533,13 @@ async def _async_main():
530533
# original training exception. Codex Phase 7 review MEDIUM.
531534
try:
532535
shutdown_refs = [
533-
pipe.shutdown_hard.remote() for _, _, _, pipe, _, _, _ in handles
536+
h.pipeline.shutdown_hard.remote() for h in handles
534537
]
535538
ray.get(shutdown_refs, timeout=60.0)
536-
for pid, _, _, _, _, _, _ in handles:
539+
for h in handles:
537540
logger.info(
538541
"[run_miles_dual] shutdown_hard complete pipeline_id=%s",
539-
pid,
542+
h.pipeline_id,
540543
)
541544
except Exception as exc: # noqa: BLE001
542545
logger.warning(

0 commit comments

Comments
 (0)