Skip to content

Commit 04fa22b

Browse files
committed
[rl] Revert generator initialization race condition fix (#3809)
Reverts the initialization reordering from #3809, which spawned the trainer first and moved TorchStore init + policy_version read before generator spawn. Restores the original ordering: spawn trainer and generators together, then init TorchStore and read policy_version.
1 parent 77444f3 commit 04fa22b

1 file changed

Lines changed: 22 additions & 30 deletions

File tree

torchtitan/experiments/rl/controller.py

Lines changed: 22 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -532,11 +532,7 @@ async def setup_async(
532532
for generator_mesh in generator_meshes:
533533
await setup_torch_elastic_env_async(generator_mesh)
534534

535-
# Spawn the trainer first; generators wait until it is ready (see below).
536-
# A generator's first MoE dispatch (vLLM warm-up in __init__) can race the
537-
# trainer's model build + weight load and fault a partial-NVLink-domain
538-
# HybridEP generator (cudaErrorIllegalAddress, hybrid_ep_backend.cuh:5693),
539-
# so sequencing keeps that first dispatch on a quiescent system.
535+
# Spawn actors on their respective meshes
540536
self.trainer = trainer_mesh.spawn(
541537
"trainer",
542538
PolicyTrainer,
@@ -548,31 +544,7 @@ async def setup_async(
548544
output_dir=config.dump_folder,
549545
)
550546

551-
# Initialize TorchStore for weight sync between trainer and generator.
552-
# StorageVolumes are spawned on the trainer mesh so they are colocated
553-
# with the weight source for faster data access in the non-RDMA path.
554-
# LocalRankStrategy: routes each process to a storage volume based on
555-
# LOCAL_RANK, so colocated processes share the same volume.
556-
# https://github.com/meta-pytorch/torchstore
557-
with sl.log_trace_span("torchstore_init"):
558-
await ts.initialize(mesh=trainer_mesh, strategy=ts.LocalRankStrategy())
559-
560-
# Barrier on trainer readiness BEFORE spawning generators (see spawn comment):
561-
# returns only after the trainer's __init__ (model build + checkpoint load), so
562-
# generators init on a quiescent system. Also reads the restored policy_version
563-
# (0 if fresh) for resume.
564-
# TODO(resume): only model/optimizer/policy_version are restored; the rollout
565-
# buffer (in-flight rollouts) and dataset stream position are NOT -- a resumed
566-
# run refills the buffer and re-reads data from the start.
567-
# TODO: investigate why we need to spawn generator later
568-
self.start_step = self._get_rank_0_value(
569-
await self.trainer.get_policy_version.call()
570-
)
571-
if self.start_step > 0:
572-
logger.info(f"Resuming RL training from step {self.start_step}")
573-
574-
# TODO: torch.compile with aot_eager backend (inductor crashes the vLLM engine on the shared model path).
575-
with sl.log_trace_span("mesh_spawn_generators"):
547+
# TODO: torch.compile with aot_eager backend (inductor crashes the vLLM engine on the shared model path).
576548
generators = []
577549
for idx, generator_mesh in enumerate(generator_meshes):
578550
actor_name = (
@@ -591,6 +563,26 @@ async def setup_async(
591563
generators.append(generator)
592564
self.generator_router = config.generator_router.build(generators=generators)
593565

566+
# Initialize TorchStore for weight sync between trainer and generator.
567+
# StorageVolumes are spawned on the trainer mesh so they are colocated
568+
# with the weight source for faster data access in the non-RDMA path.
569+
# LocalRankStrategy: routes each process to a storage volume based on
570+
# LOCAL_RANK, so colocated processes share the same volume.
571+
# https://github.com/meta-pytorch/torchstore
572+
with sl.log_trace_span("torchstore_init"):
573+
await ts.initialize(mesh=trainer_mesh, strategy=ts.LocalRankStrategy())
574+
575+
# Resume: __init__ ran CheckpointManager.load(); read back the restored policy_version
576+
# (0 if fresh) so the loop resumes at the right step and generators pull at that version.
577+
# TODO(resume): only model/optimizer/policy_version are restored. The active-slot rollout
578+
# buffer (in-flight rollouts) and the dataset stream position are NOT restored -- a resumed
579+
# run refills the buffer and re-reads data from the start. Need to recycle prompts.
580+
self.start_step = self._get_rank_0_value(
581+
await self.trainer.get_policy_version.call()
582+
)
583+
if self.start_step > 0:
584+
logger.info(f"Resuming RL training from step {self.start_step}")
585+
594586
# Initial weight sync: only the trainer loads weights; generators pull at start_step.
595587
with sl.log_trace_span("trainer_push_model_state_dict"):
596588
await self.trainer.push_model_state_dict.call()

0 commit comments

Comments
 (0)