diff --git a/3rdparty/Gym-workspace/Gym b/3rdparty/Gym-workspace/Gym index 0edca0591d5..5cd5c0d8388 160000 --- a/3rdparty/Gym-workspace/Gym +++ b/3rdparty/Gym-workspace/Gym @@ -1 +1 @@ -Subproject commit 0edca0591d5330c73054b3ca5b1229ec7a18a676 +Subproject commit 5cd5c0d8388c48f2d859a9c4ac9229ed9875e70d diff --git a/docs/assets/token-capture-ledger-queue-data-flow.dot b/docs/assets/token-capture-ledger-queue-data-flow.dot new file mode 100644 index 00000000000..9a6e55df3d2 --- /dev/null +++ b/docs/assets/token-capture-ledger-queue-data-flow.dot @@ -0,0 +1,306 @@ +digraph custody_spine { + graph [ + rankdir=TB, + newrank=true, + bgcolor="#fbfbfd", + pad="0.35", + nodesep="0.62", + ranksep="0.72", + splines=line, + fontname="Helvetica", + labelloc="t", + label="Where rollout data goes — Megatron vs vLLM", + fontsize=25 + ]; + + node [ + shape=box, + style="rounded,filled", + fillcolor="white", + color="#475569", + penwidth=1.4, + fontname="Helvetica", + fontsize=10, + margin="0.14,0.10" + ]; + + edge [ + color="#475569", + penwidth=1.7, + arrowsize=0.8, + fontname="Helvetica", + fontsize=9 + ]; + + legend [ + shape=note, + fillcolor="#f1f5f9", + color="#94a3b8", + label="FOLLOW ONE COMPLETE ROUTE\nMEGATRON: M1–M4 → B1–B2 → M5–M8 → B3–B5\nvLLM: V1–V6 → B1–B5\n\nOrange M = Megatron only · Purple V = vLLM only\nBlue B = BOTH backends; blue is required, not a separate path.\n\nDashed boxes show the backend worker's /v1/chat/completions.\nThe capture handler runs after that backend request returns." + ]; + + sample [ + shape=note, + fillcolor="#e0f2fe", + color="#0284c7", + label="GENERATION RECORD USED IN EVERY BOX\nREQUEST: prompt token IDs [10,11]\nRESPONSE: text \"done\" · token IDs [12] · log probability -0.20\nROLLOUT ID: rollout-0\nMODEL CALL ID: call-1 (the agent's first model call)\nREWARD: 1.0" + ]; + + rule [ + shape=note, + fillcolor="#fefce8", + color="#ca8a04", + label="THE ONE IMPORTANT DIFFERENCE\nvLLM: the generation record reaches TQ BEFORE the agent sees \"done\".\nMegatron: the agent sees \"done\" while the record is in MInf;\nit is copied to TQ AFTER the rollout." + ]; + + { rank=same; legend; sample; rule; } + + meg_hdr [ + group="left", + color="#ea580c", + fillcolor="#ffedd5", + penwidth=2.2, + fontsize=18, + label="MEGATRON" + ]; + + store_hdr [ + group="center", + color="#475569", + fillcolor="#f8fafc", + penwidth=2.2, + fontsize=18, + label="SHARED STORAGE" + ]; + + vllm_hdr [ + group="right", + color="#7c3aed", + fillcolor="#ede9fe", + penwidth=2.2, + fontsize=18, + label="vLLM" + ]; + + // ── Megatron processes: left column ────────────────────────────────── + m_engine [ + group="left", + color="#ea580c", + fillcolor="#ffedd5", + label="1. GENERATE THE ANSWER\nCreates the generation record:\nREQUEST: prompt token IDs [10,11]\nRESPONSE: text \"done\" · token IDs [12] · log probability -0.20\n\nSaves the record in MInf.\nSends the response text + token IDs to the capture handler.\n\nDynamicInferenceEngine" + ]; + + m_handler [ + group="left", + color="#ea580c", + fillcolor="#ffedd5", + label="AFTER THE MEGATRON WORKER RETURNS\n2. RECORD WHERE THE TOKENS ARE\nReceives \"done\" + token IDs.\nWrites a pointer to Gym:\n \"tokens are in the temporary MInf row for call-1\"\nRemoves token IDs before returning the answer.\n\nMegatronLedgerCaptureHandler" + ]; + + m_agent [ + group="left", + color="#16a34a", + fillcolor="#dcfce7", + penwidth=2.0, + label="3. GYM RETURNS TO THE AGENT\nAgent receives \"done\" with no private token fields.\nThe generation record is still in MInf.\nNothing has been written to TQ yet." + ]; + + m_flush [ + group="left", + color="#ea580c", + fillcolor="#ffedd5", + label="MEGATRON-ONLY DETOUR AFTER B2\nM5 IN: pending receipt points to MInf\nM6: load the request/response generation record from MInf\nM7: save the record in TQ; delete the temporary MInf copy\nM8 OUT: updated receipt now points to TQ\n\nMegatronGeneration / MegatronPolicyWorker" + ]; + + // ── Central custody spine ───────────────────────────────────────────── + minf [ + group="center", + shape=cylinder, + style="filled", + color="#ea580c", + fillcolor="#fed7aa", + penwidth=2.2, + label="MInf — TEMPORARY GENERATION-RECORD STORE\nUsed only by Megatron.\n\nCOMES IN\nGeneration record for model call-1:\nREQUEST: prompt token IDs [10,11]\nRESPONSE: text \"done\" · token IDs [12] · log probability -0.20\n\nGOES OUT\nThe same request/response record, loaded after the rollout.\nThe row is deleted once TQ confirms its copy is safe." + ]; + + tq_stage [ + group="center", + shape=cylinder, + style="filled", + color="#0f766e", + fillcolor="#99f6e4", + penwidth=2.2, + label="TQ STAGING — DURABLE GENERATION-RECORD STORE\nOne saved row per model call.\n\nCOMES IN\nREQUEST: prompt token IDs [10,11]\nRESPONSE: text \"done\" · token IDs [12] · log probability -0.20\nStored as tokens [10,11,12], answer mask [0,0,1],\nand aligned log probabilities [0,0,-0.20].\n\nGOES OUT\nPointer rollout-0/call-1, confirming the save succeeded.\nThe pointer format is: rollout ID / model-call ID.\nLater, the finalizer loads this row." + ]; + + gym [ + group="center", + shape=cylinder, + style="filled", + color="#2563eb", + fillcolor="#dbeafe", + penwidth=2.2, + label="GYM LEDGER — THE INDEX\nGym stores pointers, not token arrays.\n\nCOMES IN\nvLLM: \"call-1 → TQ row rollout-0/call-1\"\nMegatron: \"call-1 → its temporary row in MInf\"\nAlso stores call identity, lengths, and integrity checks.\n\nGOES OUT\nThe list of pointers for every model call in the rollout." + ]; + + nemo_gym [ + group="center", + color="#2563eb", + fillcolor="#dbeafe", + label="BOTH BACKENDS MEET HERE\nAfter the agent finishes, read every model-call pointer from Gym.\nBundle those pointers into one receipt for the rollout.\n\nNemoGym" + ]; + + receipt [ + group="center", + shape=note, + color="#0f766e", + fillcolor="#ecfeff", + penwidth=2.0, + label="ROLLOUT RECEIPT — ROUTING POINT\nContains pointers, not token arrays.\n\nvLLM: already points to TQ → continue directly to B3\nMegatron: still points to MInf → take detour M5–M8\nAfter M8, Megatron returns here with a TQ pointer.\n\nOnly a receipt whose model calls all point to TQ may continue to B3." + ]; + + finalizer [ + group="center", + color="#0369a1", + fillcolor="#e0f2fe", + penwidth=1.8, + label="FINALIZER — BUILD ONE TRAINING SAMPLE\n1. Read the TQ pointer from the receipt.\n2. Load the request/response generation record from TQ staging.\n3. Check it, attach reward 1.0, and make tensors.\n4. Save the completed sample in TQ replay.\n\nBlackboxFinalizer / TQTokenSource" + ]; + + tq_replay [ + group="center", + shape=cylinder, + style="filled", + color="#0369a1", + fillcolor="#bae6fd", + penwidth=2.2, + label="TQ REPLAY — READY FOR TRAINING\n\nCOMES IN\nThe completed sample:\ntokens [10,11,12] · answer mask [0,0,1]\nlog probabilities [0,0,-0.20] · reward 1.0\n\nGOES OUT\nA model-ready rollout batch for the training step." + ]; + + sampler [ + group="center", + color="#0369a1", + fillcolor="#e0f2fe", + label="ROLLOUT IS RETURNED\nThe replay buffer marks the sample ready.\nThe sampler returns it as part of the rollout batch.\n\nTQReplayBuffer / BaseSampler" + ]; + + // ── vLLM processes: right column ───────────────────────────────────── + v_worker [ + group="right", + color="#7c3aed", + fillcolor="#ede9fe", + label="1. GENERATE THE ANSWER\nCreates the generation record:\nREQUEST: prompt token IDs [10,11]\nRESPONSE: text \"done\" · token IDs [12] · log probability -0.20\n\nVllmAsyncGenerationWorker" + ]; + + v_capture [ + group="right", + color="#7c3aed", + fillcolor="#ede9fe", + label="2. SAVE THE RECORD BEFORE RETURNING\nWrites the request/response generation record directly to TQ.\nWaits until TQ confirms it is stored at rollout-0/call-1.\nOnly then may the HTTP response continue.\n\nRolloutTokenCapture / TQTokenSink" + ]; + + v_handler [ + group="right", + color="#7c3aed", + fillcolor="#ede9fe", + label="AFTER THE vLLM WORKER RETURNS\n3. RECORD WHERE THE TOKENS ARE\nReceives \"done\" + the TQ pointer rollout-0/call-1.\nWrites to Gym: \"call-1 → TQ row rollout-0/call-1\".\nRemoves the private pointer before returning the answer.\n\nVLLMWorkerCaptureHandler" + ]; + + v_agent [ + group="right", + color="#16a34a", + fillcolor="#dcfce7", + penwidth=2.0, + label="4. GYM RETURNS TO THE AGENT\nAgent receives response text \"done\" with no private token fields.\nThe request/response generation record is already safely stored in TQ." + ]; + + // Dashed boundaries show only the downstream generation-worker HTTP call. + // The Gym-side capture handlers intentionally remain outside these clusters. + subgraph cluster_megatron_worker_endpoint { + label="INSIDE MEGATRON GENERATION WORKER\nbackend /v1/chat/completions\nM1 writes MInf before the HTTP response returns"; + labelloc="t"; + labeljust="l"; + color="#f97316"; + fontcolor="#c2410c"; + fontname="Helvetica"; + fontsize=11; + penwidth=2.0; + style="rounded,dashed"; + margin=18; + m_engine; + } + + subgraph cluster_vllm_worker_endpoint { + label="INSIDE vLLM GENERATION WORKER\nbackend /v1/chat/completions\nV2 waits for TQ before the HTTP response returns"; + labelloc="t"; + labeljust="l"; + color="#8b5cf6"; + fontcolor="#6d28d9"; + fontname="Helvetica"; + fontsize=11; + penwidth=2.0; + style="rounded,dashed"; + margin=18; + v_worker; + v_capture; + } + + // Invisible routing points keep the two late Megatron transfers in the + // open channel between the Megatron and shared-storage columns. + m4_mid [shape=point, width=0.01, height=0.01, label="", style=invis]; + m5_mid [shape=point, width=0.01, height=0.01, label="", style=invis]; + + // ── Visible data transfers (constraint=false: grid owns placement) ─── + m_engine:e -> minf:w [color="#ea580c", fontcolor="#c2410c", constraint=false, label="M1 save request/response generation record temporarily"]; + m_handler:e -> gym:w [color="#ea580c", fontcolor="#c2410c", constraint=false, label="M2 save pointer to call-1's temporary MInf row"]; + m_handler:s -> m_agent:n [color="#ea580c", fontcolor="#c2410c", constraint=false, headlabel="M3 clean answer: \"done\"", labeldistance=3.5, labelangle=25]; + m_agent:e -> nemo_gym:w [color="#ea580c", fontcolor="#c2410c", constraint=false, label="M4 rollout finished"]; + receipt:w -> m_flush:e [color="#ea580c", fontcolor="#c2410c", constraint=false, dir=both, arrowtail=normal, label="M5 pending receipt / M8 updated receipt"]; + minf:sw -> m4_mid:n [color="#ea580c", fontcolor="#c2410c", constraint=false, arrowhead=none, label="M6 load generation record from MInf"]; + m4_mid:s -> m_flush:n [color="#ea580c", constraint=false]; + m_flush:n -> m5_mid:s [color="#ea580c", fontcolor="#c2410c", constraint=false, arrowhead=none, label="M7 save durable copy in TQ"]; + m5_mid:n -> tq_stage:sw [color="#ea580c", constraint=false]; + + v_worker:s -> v_capture:n [color="#7c3aed", fontcolor="#6d28d9", constraint=false, label="V1 request/response generation record"]; + v_capture:w -> tq_stage:e [color="#7c3aed", fontcolor="#6d28d9", constraint=false, label="V2 save generation record; wait for confirmation"]; + tq_stage:e -> v_handler:w [color="#7c3aed", fontcolor="#6d28d9", constraint=false, headlabel="V3 answer + TQ pointer rollout-0/call-1", labeldistance=4.0, labelangle=20]; + v_handler:w -> gym:e [color="#7c3aed", fontcolor="#6d28d9", constraint=false, label="V4 save pointer to TQ row rollout-0/call-1"]; + v_handler:s -> v_agent:n [color="#7c3aed", fontcolor="#6d28d9", constraint=false, headlabel="V5 clean answer: \"done\"", labeldistance=3.5, labelangle=-25]; + v_agent:w -> nemo_gym:e [color="#7c3aed", fontcolor="#6d28d9", constraint=false, label="V6 rollout finished"]; + + gym:s -> nemo_gym:n [color="#2563eb", fontcolor="#1d4ed8", constraint=false, taillabel="B1 BOTH", labeldistance=3.5, labelangle=-25]; + nemo_gym:s -> receipt:n [color="#2563eb", fontcolor="#1d4ed8", constraint=false, label="B2 BOTH: build rollout receipt"]; + receipt:s -> finalizer:n [color="#0369a1", fontcolor="#0369a1", constraint=false, label="B3 BOTH: receipt is ready; all calls point to TQ"]; + finalizer:s -> tq_replay:n [color="#0369a1", fontcolor="#0369a1", constraint=false, label="B4 BOTH: save completed training sample"]; + tq_replay:s -> sampler:n [color="#0369a1", fontcolor="#0369a1", constraint=false, label="B5 BOTH: return model-ready rollout batch"]; + + // ── Invisible placement grid: strictly Megatron | stores | vLLM ───── + node [shape=point, width=0.01, height=0.01, label="", style=invis]; + l2 [group="left"]; l5 [group="left"]; l6 [group="left"]; l8 [group="left"]; + r5 [group="right"]; r6 [group="right"]; r8 [group="right"]; + node [shape=box, style="rounded,filled", width=0, height=0]; + + { rank=same; meg_hdr; store_hdr; vllm_hdr; } + { rank=same; m_engine; minf; v_worker; } + { rank=same; l2; tq_stage; v_capture; } + { rank=same; m_handler; gym; v_handler; } + { rank=same; m_agent; m4_mid; m5_mid; nemo_gym; v_agent; } + { rank=same; m_flush; receipt; r5; } + { rank=same; l6; finalizer; r6; } + { rank=same; l8; tq_replay; r8; } + + meg_hdr -> store_hdr -> vllm_hdr [style=invis, weight=1000]; + m_engine -> minf -> v_worker [style=invis, weight=1000]; + l2 -> tq_stage -> v_capture [style=invis, weight=1000]; + m_handler -> gym -> v_handler [style=invis, weight=1000]; + m_agent -> m4_mid -> m5_mid -> nemo_gym -> v_agent [style=invis, weight=1000]; + m_flush -> receipt -> r5 [style=invis, weight=1000]; + l6 -> finalizer -> r6 [style=invis, weight=1000]; + l8 -> tq_replay -> r8 [style=invis, weight=1000]; + + meg_hdr -> m_engine -> l2 -> m_handler -> m_agent -> m_flush -> l6 -> l8 [style=invis, weight=1000]; + store_hdr -> minf -> tq_stage -> gym -> nemo_gym -> receipt -> finalizer -> tq_replay -> sampler [style=invis, weight=1000]; + vllm_hdr -> v_worker -> v_capture -> v_handler -> v_agent -> r5 -> r6 -> r8 [style=invis, weight=1000]; + + sample -> store_hdr [style=invis, weight=1000]; +} diff --git a/docs/assets/token-capture-ledger-queue-data-flow.png b/docs/assets/token-capture-ledger-queue-data-flow.png new file mode 100644 index 00000000000..0da219523ba Binary files /dev/null and b/docs/assets/token-capture-ledger-queue-data-flow.png differ diff --git a/docs/guides/single-controller.md b/docs/guides/single-controller.md index 703859d2c8c..82b3174f485 100644 --- a/docs/guides/single-controller.md +++ b/docs/guides/single-controller.md @@ -25,7 +25,7 @@ uv run examples/run_grpo_single_controller.py --config enabled: true ``` -2. **Enable vLLM async engine** and **disable colocated inference** (SC drives rollout via `RolloutManager.generate_and_push`, which is only supported on the disaggregated async engine): +2. **Pick a generation backend**. With vllm, **disable colocated inference** and enable the async engine (SC drives rollout via `RolloutManager.generate_and_push`, which is only supported on the disaggregated async engine): ```yaml policy: @@ -40,6 +40,19 @@ uv run examples/run_grpo_single_controller.py --config gpus_per_node: 4 # inference GPUs; remainder go to training ``` + Megatron generation is also supported, colocated or non-colocated. It requires the Megatron trainer (`policy.megatron_cfg.enabled: true`) and NeMo-Gym rollouts additionally require `policy.generation.mcore_generation_config.expose_http_server: true`. Colocated (`colocated.enabled: true`) additionally requires `async_rl.max_buffered_rollouts >= grpo.num_prompts_per_step`, to avoid switching from generation to training when a full batch is not available. + The non-colocated exemplar — a NeMo-Gym run with the OpenAI server exposed — lives at [examples/nemo_gym/grpo_qwen3_0_6b_megatron_generation_single_controller.yaml](../../examples/nemo_gym/grpo_qwen3_0_6b_megatron_generation_single_controller.yaml); the colocated exemplar at [examples/configs/grpo_math_1B_megatron_generation_colocated_single_controller.yaml](../../examples/configs/grpo_math_1B_megatron_generation_colocated_single_controller.yaml): + + ```yaml + policy: + megatron_cfg: + enabled: true + generation: + backend: "megatron" + colocated: + enabled: true + ``` + 3. **One RL step = one optimizer step.** The SC train pump does not support multi-mini-step inside a single RL step (see `validate_single_controller_config` in [nemo_rl/algorithms/single_controller_utils/config.py](../../nemo_rl/algorithms/single_controller_utils/config.py)): ```python @@ -171,7 +184,7 @@ SC reads its async knobs from `async_rl:` and **requires `grpo.async_grpo: null` The SC path is still under active development. Feature gaps are tracked in [issue #2625](https://github.com/NVIDIA-NeMo/RL/issues/2625). Notable items: - Train backend: only Megatron is supported and validated; the AutoModel training path has not been tested on SC. -- Generation backend: only vLLM is supported and validated; Megatron generation, SGLang, and TRT-LLM have not been tested on SC. -- Checkpointing and validation are not yet supported (setup raises if enabled). +- Generation backend: vLLM and Megatron generation are supported (Megatron in both non-colocated and colocated modes); SGLang and TRT-LLM have not been tested on SC. +- Validation is not yet supported (setup raises if enabled); checkpointing is. - The `windowed` sampler has no `over_sampling_ratio` cap — over-produced groups aged past the window are evicted, wasting rollout compute. - The drain gate in refit is not yet supported. diff --git a/examples/configs/grpo_math_1B_megatron_generation_colocated_single_controller.yaml b/examples/configs/grpo_math_1B_megatron_generation_colocated_single_controller.yaml new file mode 100644 index 00000000000..680c9143f7c --- /dev/null +++ b/examples/configs/grpo_math_1B_megatron_generation_colocated_single_controller.yaml @@ -0,0 +1,11 @@ +# SingleController with the Megatron generation backend (colocated). +defaults: grpo_math_1B_megatron_single_controller.yaml + +policy: + generation: + backend: megatron + colocated: + enabled: true + resources: + gpus_per_node: null + num_nodes: null diff --git a/examples/nemo_gym/grpo_qwen3_0_6b_megatron_generation_single_controller.yaml b/examples/nemo_gym/grpo_qwen3_0_6b_megatron_generation_single_controller.yaml new file mode 100644 index 00000000000..df166589972 --- /dev/null +++ b/examples/nemo_gym/grpo_qwen3_0_6b_megatron_generation_single_controller.yaml @@ -0,0 +1,70 @@ +# GRPO on the NeMo-Gym workplace-assistant environment via the SingleController path, +# using non-colocated Megatron Inference. +# Gym rollouts go through the persistent Megatron engine's OpenAI-compatible server, +# Smoke-test scale; Qwen3-0.6B on one node with 2 GPUs (1 training + 1 generation); +# the CI-run variant of this setup is +# tests/functional/grpo_megatron_generation_gym_single_controller.sh. +defaults: "grpo_qwen3_30ba3b_instruct.yaml" + +grpo: + # SC requires one optimizer step per RL step: + # num_prompts_per_step * num_generations_per_prompt == policy.train_global_batch_size + num_prompts_per_step: 4 + num_generations_per_prompt: 2 + max_num_steps: 10 # short demo; raise for a real run + # SC does not support validation yet (setup raises when it is enabled). + val_period: 0 + val_at_start: false + # SC replaces the legacy async-GRPO path. + async_grpo: null + +policy: + model_name: Qwen/Qwen3-0.6B + train_global_batch_size: 8 + # Full workplace-assistant prompts (all tools attached) run past 4k tokens. + max_total_sequence_length: 8192 + + megatron_cfg: + tensor_model_parallel_size: 1 + expert_model_parallel_size: 1 + context_parallel_size: 1 + sequence_parallel: false + + generation: + backend: "megatron" + mcore_generation_config: + # NeMo-Gym drives rollouts through the engine's OpenAI server. + expose_http_server: true + colocated: + enabled: false + resources: + num_nodes: 1 + gpus_per_node: 1 + +# The gym family sets no data_plane block, so the full one is spelled out here +# (values as documented in examples/configs/grpo_math_1B.yaml). +data_plane: + enabled: true + impl: transfer_queue + backend: "simple" # TQ storage backend ('simple' or 'mooncake_cpu') + storage_capacity: 1000000 # max samples retained per partition + num_storage_units: 2 # storage shards + claim_meta_poll_interval_s: 0.5 # blocking-claim poll cadence + global_segment_size: 549755813888 # 512 GiB — used when backend == "mooncake_cpu" + local_buffer_size: 68719476736 # 64 GiB — used when backend == "mooncake_cpu" + +async_rl: + sampler: + name: in_order + # 0 = fully synchronous. No off-policy data is admitted, so the inherited + # use_importance_sampling_correction=false stays valid. + max_lookahead_versions: 0 + min_groups_for_streaming_train: ${grpo.num_prompts_per_step} + max_inflight_prompts: ${grpo.num_prompts_per_step} + +checkpointing: + # SC does not support checkpointing yet (setup raises when it is enabled). + enabled: false + +cluster: + gpus_per_node: 2 diff --git a/nemo_rl/algorithms/grpo.py b/nemo_rl/algorithms/grpo.py index 9bf15717cd4..18347bde91a 100644 --- a/nemo_rl/algorithms/grpo.py +++ b/nemo_rl/algorithms/grpo.py @@ -1040,11 +1040,6 @@ def _spinup_nemo_gym(base_urls, model_name): # vllm model loading prefers clean environment, initialize policy_generation before policy in colocated mode backend = generation_config["backend"] - gen_init_time_key = ( - "megatron_generation_init_time_s" - if backend == "megatron" - else f"{backend}_init_time_s" - ) generation_config["model_name"] = policy_config["model_name"] # Needed for vLLM generation_config["_debug_payload_metrics"] = grpo_config.debug_payload_metrics remote_transport = None @@ -1100,9 +1095,13 @@ def _spinup_nemo_gym(base_urls, model_name): # plane exists. Default is the plain Policy class — legacy behavior. _make_policy = policy_factory if policy_factory is not None else Policy - def init_policy(): + def init_policy(reserved_http_server_port: Optional[int] = None): """Initialize policy training workers.""" t0 = time.perf_counter() + extra_policy_kwargs = {} + if reserved_http_server_port is not None: + # Colocated Megatron generation serves HTTP from the training workers. + extra_policy_kwargs["reserved_http_server_port"] = reserved_http_server_port p = _make_policy( cluster=train_cluster, config=policy_config, @@ -1112,6 +1111,7 @@ def init_policy(): optimizer_path=optimizer_path, init_optimizer=True, init_reference_model=init_reference_model, + **extra_policy_kwargs, ) # Keep custom policy_factory call signatures backward compatible. p.debug_payload_metrics = grpo_config.debug_payload_metrics @@ -1139,25 +1139,21 @@ def init_sglang(): pg.finish_generation() return pg, time.perf_counter() - t0 - def init_megatron_generation(policy=None): + def init_megatron_generation( + policy=None, reserved_http_server_port: Optional[int] = None + ): """Initialize Megatron generation.""" t0 = time.perf_counter() - if colocated_inference: - mg = MegatronGeneration( - policy=policy, - config=policy_config, - tokenizer=tokenizer, - processor=processor, - ) - else: - mg = MegatronGeneration( - cluster=inference_cluster, - config=policy_config, - tokenizer=tokenizer, - processor=processor, - weights_path=weights_path, - skip_weight_load=True, - ) + mg = MegatronGeneration( + config=policy_config, + tokenizer=tokenizer, + cluster=None if colocated_inference else inference_cluster, + policy=policy if colocated_inference else None, + processor=processor, + weights_path=weights_path, + skip_weight_load=not colocated_inference, + reserved_http_server_port=reserved_http_server_port, + ) return mg, time.perf_counter() - t0 def initialize_generation_with_policy( @@ -1195,7 +1191,7 @@ def initialize_generation_with_policy( parallel_wall_time = time.perf_counter() - parallel_start_time # Store timing metrics - setattr(setup_timing_metrics, gen_init_time_key, generation_time) + setup_timing_metrics.generation_init_time_s = generation_time setup_timing_metrics.policy_init_time_s = policy_time setup_timing_metrics.parallel_wall_time_s = parallel_wall_time setup_timing_metrics.parallel_init_enabled = 1.0 @@ -1209,7 +1205,7 @@ def initialize_generation_with_policy( # Initialize generation engine first (clean GPU memory), then policy policy_generation, generation_time = init_generation_fn() - setattr(setup_timing_metrics, gen_init_time_key, generation_time) + setup_timing_metrics.generation_init_time_s = generation_time policy, policy_time = init_policy() setup_timing_metrics.policy_init_time_s = policy_time @@ -1219,22 +1215,79 @@ def initialize_generation_with_policy( # Handle generation-specific setup if backend == "megatron": - # Initialize training first so checkpoint conversion completes before inference starts. - policy, policy_time = init_policy() - setup_timing_metrics.policy_init_time_s = policy_time + if enable_nemo_gym: + print( + " ⚡ Reserving the Megatron server address for overlapped NeMo Gym init", + flush=True, + ) + reserve_t0 = time.perf_counter() + reserved_url, reserved_http_server_port, port_holder = ( + MegatronGeneration.reserve_http_server_address( + train_cluster if colocated_inference else inference_cluster, + policy_config, + ) + ) + reserve_time = time.perf_counter() - reserve_t0 + setup_timing_metrics.generation_init_reserve_time_s = reserve_time + print(f" ✓ Reserved Megatron server URL: {reserved_url}", flush=True) + + def init_megatron_stack(): + """Init policy then generation; rank 0 holds the reserved port.""" + p, policy_t = init_policy( + reserved_http_server_port=reserved_http_server_port + if colocated_inference + else None + ) + pg, gen_t = init_megatron_generation( + p, + reserved_http_server_port=None + if colocated_inference + else reserved_http_server_port, + ) + return p, policy_t, pg, gen_t - # Colocated wraps the training policy; non-colocated builds a dedicated inference policy. - policy_generation, megatron_gen_time = init_megatron_generation(policy) - setup_timing_metrics.megatron_generation_init_time_s = megatron_gen_time + def init_nemo_gym(): + """Spin up NeMo Gym servers against the reserved URL.""" + return _spinup_nemo_gym([reserved_url], generation_config["model_name"]) - if enable_nemo_gym: - # The Megatron inference engine must be up before its server URLs exist. - nemo_gym_actor, nemo_gym_time = _spinup_nemo_gym( - policy_generation.dp_openai_server_base_urls, - generation_config["model_name"], + init_tasks = { + "megatron": init_megatron_stack, + "nemo_gym": init_nemo_gym, + } + print(f" ⚡ Init tasks: {', '.join(init_tasks.keys())}", flush=True) + try: + with ThreadPoolExecutor(max_workers=len(init_tasks)) as executor: + submitted = {k: executor.submit(fn) for k, fn in init_tasks.items()} + results = {k: f.result() for k, f in submitted.items()} + finally: + ray.kill(port_holder) + + policy, policy_time, policy_generation, megatron_gen_time = results[ + "megatron" + ] + nemo_gym_actor, nemo_gym_time = results["nemo_gym"] + setup_timing_metrics.policy_init_time_s = policy_time + setup_timing_metrics.generation_init_time_s = ( + reserve_time + megatron_gen_time ) + setup_timing_metrics.generation_init_load_time_s = megatron_gen_time setup_timing_metrics.nemo_gym_init_time_s = nemo_gym_time + served_urls = policy_generation.dp_openai_server_base_urls + if served_urls != [reserved_url]: + raise RuntimeError( + "Megatron server came up at a different address than the one " + f"pre-published to NeMo Gym: reserved {reserved_url}, serving {served_urls}." + ) + else: + # Initialize training first so checkpoint conversion completes before inference starts. + policy, policy_time = init_policy() + setup_timing_metrics.policy_init_time_s = policy_time + + # Colocated wraps the training policy; non-colocated builds a dedicated inference policy. + policy_generation, megatron_gen_time = init_megatron_generation(policy) + setup_timing_metrics.generation_init_time_s = megatron_gen_time + print( f" ✓ Using {backend} backend for generation with {policy_config['model_name']}", flush=True, @@ -1356,7 +1409,11 @@ def init_vllm_then_policy(): policy_generation, vllm_load_time = results["vllm"] policy, policy_time = results["policy"] nemo_gym_actor, nemo_gym_time = results["nemo_gym"] - setup_timing_metrics.vllm_init_time_s = vllm_reserve_time + vllm_load_time + setup_timing_metrics.generation_init_time_s = ( + vllm_reserve_time + vllm_load_time + ) + setup_timing_metrics.generation_init_reserve_time_s = vllm_reserve_time + setup_timing_metrics.generation_init_load_time_s = vllm_load_time setup_timing_metrics.policy_init_time_s = policy_time setup_timing_metrics.nemo_gym_init_time_s = nemo_gym_time else: @@ -1445,8 +1502,25 @@ def init_trtllm(): "https://github.com/NVIDIA-NeMo/RL/issues/3288." ) + if backend == "megatron": + t0 = time.perf_counter() + policy_generation.weight_synchronizer = create_weight_synchronizer( + policy=policy, + generation=policy_generation, + generation_backend=backend, + colocated=colocated_inference, + train_cluster=train_cluster, + inference_cluster=None if colocated_inference else inference_cluster, + ) + policy_generation.weight_synchronizer.init_communicator() + setup_timing_metrics.collective_init_time_s = time.perf_counter() - t0 + if not colocated_inference: + # Load the model weights now. + t0 = time.perf_counter() + policy_generation.weight_synchronizer.sync_weights() + setup_timing_metrics.weight_sync_time_s = time.perf_counter() - t0 # if it is not colocated inference, initialize collective communication for update weights - if ( + elif ( not colocated_inference and remote_transport is None and checkpoint_engine_config is None @@ -1576,7 +1650,7 @@ def init_trtllm(): ) # Log worker initialization timing metrics to logger - print_setup_timing_summary(setup_timing_metrics, gen_init_time_key) + print_setup_timing_summary(setup_timing_metrics) logger.log_metrics( setup_timing_metrics.to_metrics_dict(), step=0, prefix="timing/setup" ) diff --git a/nemo_rl/algorithms/metric_utils.py b/nemo_rl/algorithms/metric_utils.py index 8c7cd7c0ffe..2384521db98 100644 --- a/nemo_rl/algorithms/metric_utils.py +++ b/nemo_rl/algorithms/metric_utils.py @@ -22,20 +22,17 @@ class SetupTimingMetrics: """Driver-side per-phase timings collected during setup.""" - # grpo.py only: generation-backend init (exactly one populated per run). - vllm_init_time_s: Optional[float] = None - sglang_init_time_s: Optional[float] = None - trtllm_init_time_s: Optional[float] = None - megatron_generation_init_time_s: Optional[float] = None - - # SC only: generation init (reserve + load). + # Generation-backend init. generation_init_time_s: Optional[float] = None + # When overlapping NeMo Gym init, the total decomposes as reserve + load. generation_init_reserve_time_s: Optional[float] = None generation_init_load_time_s: Optional[float] = None policy_init_time_s: Optional[float] = None nemo_gym_init_time_s: Optional[float] = None collective_init_time_s: Optional[float] = None + # Non-colocated megatron's post-init weight sync into the engine. + weight_sync_time_s: Optional[float] = None # Non-colocated only. (grpo.py only) parallel_wall_time_s: Optional[float] = None @@ -64,34 +61,24 @@ def to_metrics_dict(self) -> dict[str, Any]: return base -def print_setup_timing_summary( - metrics: SetupTimingMetrics, gen_init_time_key: Optional[str] = None -) -> None: +def print_setup_timing_summary(metrics: SetupTimingMetrics) -> None: """Print the setup-phase summary block. Args: metrics: Populated timing metrics. - gen_init_time_key: grpo.py passes the backend-specific field name - (e.g. "vllm_init_time_s"); SC leaves it None and the summary - reads generation_init_time_s (+ optional reserve/load split). """ print("\n▶ Worker Initialization Timing:") + assert metrics.generation_init_time_s is not None if metrics.generation_init_reserve_time_s: - # SC + gym-on path + # gym-on: an address was reserved, so the total decomposes as reserve + load. print( f" Generation init: {metrics.generation_init_time_s:.1f}s" f" (reserve {metrics.generation_init_reserve_time_s:.1f}s" f" + load {metrics.generation_init_load_time_s:.1f}s)" ) - elif gen_init_time_key is None: - # SC + gym-off path - assert metrics.generation_init_time_s is not None - print(f" Generation init: {metrics.generation_init_time_s:.1f}s") else: - # grpo.py path - assert metrics.generation_init_time_s is None - print(f" Generation init: {getattr(metrics, gen_init_time_key):.1f}s") + print(f" Generation init: {metrics.generation_init_time_s:.1f}s") print(f" Policy init: {metrics.policy_init_time_s:.1f}s") diff --git a/nemo_rl/algorithms/ppo.py b/nemo_rl/algorithms/ppo.py index 30374bbb313..567ef1320da 100644 --- a/nemo_rl/algorithms/ppo.py +++ b/nemo_rl/algorithms/ppo.py @@ -744,7 +744,6 @@ def init_sglang(): def initialize_generation_with_policy( init_generation_fn, generation_name: str, - init_time_key: str, worker_init_timing_metrics: dict, ): """Generic function to initialize a generation engine (vLLM or SGLang) along with policy. @@ -752,7 +751,6 @@ def initialize_generation_with_policy( Args: init_generation_fn: Function that initializes the generation engine (init_vllm or init_sglang) generation_name: Name of the generation engine ("vLLM" or "SGLang") - init_time_key: Key name for storing initialization time in metrics ("vllm_init_time_s" or "sglang_init_time_s") worker_init_timing_metrics: Dictionary to store timing metrics Returns: @@ -763,7 +761,7 @@ def initialize_generation_with_policy( # Policy and value initialize serially because they share training GPUs. policy_generation, generation_time = init_generation_fn() - worker_init_timing_metrics[init_time_key] = generation_time + worker_init_timing_metrics["generation_init_time_s"] = generation_time policy, policy_time = init_policy() # Block until the policy worker's __init__ completes and offload to @@ -821,7 +819,6 @@ def initialize_generation_with_policy( policy_generation, policy, value_model = initialize_generation_with_policy( init_generation_fn=init_vllm, generation_name="vLLM", - init_time_key="vllm_init_time_s", worker_init_timing_metrics=worker_init_timing_metrics, ) @@ -840,7 +837,6 @@ def initialize_generation_with_policy( policy_generation, policy, value_model = initialize_generation_with_policy( init_generation_fn=init_sglang, generation_name="SGLang", - init_time_key="sglang_init_time_s", worker_init_timing_metrics=worker_init_timing_metrics, ) @@ -892,12 +888,12 @@ def initialize_generation_with_policy( if worker_init_timing_metrics: print("\n▶ Worker Initialization Timing:") - vllm_time = worker_init_timing_metrics.get("vllm_init_time_s", 0) + gen_time = worker_init_timing_metrics.get("generation_init_time_s", 0) policy_time = worker_init_timing_metrics.get("policy_init_time_s", 0) total_setup = worker_init_timing_metrics.get("total_setup_time_s", 0) - if vllm_time: - print(f" vLLM init: {vllm_time:.1f}s") + if gen_time: + print(f" Generation init: {gen_time:.1f}s") if policy_time: print(f" Policy init: {policy_time:.1f}s") diff --git a/nemo_rl/algorithms/single_controller.py b/nemo_rl/algorithms/single_controller.py index 70f8c0cfaba..9ec562bfeed 100644 --- a/nemo_rl/algorithms/single_controller.py +++ b/nemo_rl/algorithms/single_controller.py @@ -110,6 +110,7 @@ PromptGroupStatus, ) from nemo_rl.experience.route_plan import decode_route_plan +from nemo_rl.models.generation.megatron.megatron_generation import MegatronGeneration from nemo_rl.models.generation.sglang.sglang_generation import SGLangGeneration from nemo_rl.models.generation.vllm import VllmGeneration from nemo_rl.models.policy.tq_policy import TQPolicy @@ -120,7 +121,7 @@ if TYPE_CHECKING: from nemo_rl.experience.finalizer_actor import FinalizationRequest -Generation = Union[VllmGeneration, SGLangGeneration] +Generation = Union[VllmGeneration, SGLangGeneration, MegatronGeneration] _TELEMETRY_WALL_TIME_METRIC = "telemetry/wall_time_seconds" _TELEMETRY_PREFIXES = ( @@ -2156,6 +2157,7 @@ async def _train_pump(self) -> None: evicted_stale_prompt_groups = 0 min_sample_version = None step_open = False + generation_released = False calibration_batches: list[BatchedDataDict[Any]] = [] consumed_metas: list[KVBatchMeta] = [] consumed_group_count = 0 @@ -2205,6 +2207,9 @@ async def _train_pump(self) -> None: self._async_cfg.min_groups_for_streaming_train, max_prompt_groups, ) + if self._gen.blocks_training(): + # Always assemble a whole batch in colocated mode. + min_prompt_groups = max_prompt_groups if self._rollout_recovery_ledger is None: train_meta, num_groups = await self._sampler.select( current_train_weight=self._trainer_version, @@ -2264,6 +2269,17 @@ async def _train_pump(self) -> None: float(value) ) + # A generation engine that blocks training must stand down; + # sleep the engine to allow for the trainer to perform GPU work. + # Safe because the select above assembles whole steps for blocking engines. + # In-flight requests freeze, then continue on fresh weights. + # The post-step `_sync_weights` wake reopens the gate, + # except on save-bound steps, where the wake is deferred until after the save. + if not generation_released and self._gen.blocks_training(): + self._rollout_permitted.clear() + await asyncio.to_thread(self._gen.finish_generation) + generation_released = True + # Compute prev_logprobs / ref_logprobs if ( self._policy_logprobs_required @@ -2371,23 +2387,6 @@ async def _train_pump(self) -> None: self._trainer_version += 1 self._train_steps += 1 self._optimizer_commit_in_progress = False - with self._timer.time("weight_sync"): - calibration_data = ( - BatchedDataDict.from_batches(calibration_batches) - if calibration_batches - else None - ) - aborted_stale_inflight_groups = await self._sync_weights( - calibration_data=calibration_data - ) - step_metrics.update( - { - "evicted_stale_prompt_groups": evicted_stale_prompt_groups, - "aborted_stale_inflight_groups": aborted_stale_inflight_groups, - } - ) - - # Checkpointing (mirrors async_grpo_train's save block). self._consumed_samples += grpo_cfg.num_prompts_per_step self._total_valid_tokens += step_metrics.get("global_valid_toks", 0) self._timeout.mark_iteration() @@ -2410,13 +2409,58 @@ async def _train_pump(self) -> None: and self._train_steps % ft_save_period == 0 ) ) + # Call once per step and reuse the bool. should_save_by_timeout = self._timeout.check_save() + will_save_checkpoint = self._master_config.checkpointing[ + "enabled" + ] and (should_save_by_step or should_save_by_timeout) + # A save-bound colocated step defers the wake until after the save. + defer_wake_for_save = ( + will_save_checkpoint + and self._gen.blocks_training() + and self._gen.wake_carries_weight_updates() + ) - if self._master_config.checkpointing["enabled"] and ( - should_save_by_step or should_save_by_timeout - ): + with self._timer.time("weight_sync"): + calibration_data = ( + BatchedDataDict.from_batches(calibration_batches) + if calibration_batches + else None + ) + aborted_stale_inflight_groups = await self._sync_weights( + calibration_data=calibration_data, + defer_engine_wake=defer_wake_for_save, + ) + step_metrics.update( + { + "evicted_stale_prompt_groups": evicted_stale_prompt_groups, + "aborted_stale_inflight_groups": aborted_stale_inflight_groups, + } + ) + + if will_save_checkpoint: with self._timer.time("checkpointing"): await self._save_checkpoint(step_metrics) + if defer_wake_for_save: + # The save is done; wake the engine unless the loop is about to exit. + loop_will_exit = ( + self._train_steps >= grpo_cfg.max_num_steps + or should_save_by_timeout + or ( + self._rollout_exhausted.is_set() + and len(self._buffer) == 0 + ) + ) + if not loop_will_exit: + with self._timer.time("weight_sync"): + # The save onloaded model+optimizer; generation needs offload. + await asyncio.to_thread( + self._trainer.offload_after_refit + ) + await asyncio.to_thread( + self._gen.prepare_for_generation + ) + self._rollout_permitted.set() timing_metrics: dict[str, float] = self._timer.get_timing_metrics( reduction_op="sum" @@ -2621,8 +2665,9 @@ async def _save_checkpoint(self, step_metrics: dict[str, Any]) -> None: async def _save_checkpoint_locked(self, step_metrics: dict[str, Any]) -> None: """Write a full checkpoint for the just-finished train step. - Everything except the (possibly async) policy weight write must be - on disk before begin_finalization; rollouts keep running throughout. + Everything except the (possibly async) policy weight write must be on disk + before begin_finalization. Non-colocated engines keep serving rollouts throughout; + colocated engine is held stood-down through the save. """ save_state = self._save_state save_state.current_step = self._train_steps @@ -2764,12 +2809,12 @@ async def _sync_weights( self, *, calibration_data: Optional[BatchedDataDict[Any]] = None, + defer_engine_wake: bool = False, ) -> int: """Pause new rollout dispatches, synchronize weights, resume. - SC owns the pause gate; in-flight generations continue through the - refit — vLLM V1 async engine supports weight updates during pending - requests. + SC owns the pause gate. vLLM serves through the refit; + its async engine supports weight updates during pending requests. Flow: 1. _rollout_permitted.clear() — no new dispatches @@ -2777,9 +2822,13 @@ async def _sync_weights( 3. weight_synchronizer.sync_weights(kv_scales=...) 4. _rollout_permitted.set() — resume + `defer_engine_wake` is used to skip refit and wake on save-bound steps for colocated cases. + Args: calibration_data: Optional data used to calibrate FP8 KV-cache scales before synchronizing weights. + defer_engine_wake: Keep the engine asleep: offload the trainer's + refit buffers, stamp the version, leave the gate closed. Returns: The number of stale in-flight rollout groups aborted before the @@ -2815,12 +2864,15 @@ async def _sync_weights( ) kv_scales = calibration_result["layers"] - await asyncio.to_thread( - self._weight_synchronizer.sync_weights, - kv_scales=kv_scales, - ) + if defer_engine_wake: + await asyncio.to_thread(self._trainer.offload_before_refit) + else: + await asyncio.to_thread( + self._weight_synchronizer.sync_weights, + kv_scales=kv_scales, + ) if self._async_cfg.recompute_kv_cache_after_weight_updates: - self._gen.invalidate_kv_cache() + await asyncio.to_thread(self._gen.invalidate_kv_cache) elapsed = time.monotonic() - t0 print(f" _sync_weights: sync done in {elapsed:.3f}s", flush=True) @@ -2831,7 +2883,8 @@ async def _sync_weights( await asyncio.to_thread( self._gen.set_rollout_weight_version, self._trainer_version ) - self._rollout_permitted.set() + if not defer_engine_wake: + self._rollout_permitted.set() return aborted_stale_inflight_groups async def _log_rollout_throughput_metrics(self, *, emit: bool = True) -> None: diff --git a/nemo_rl/algorithms/single_controller_utils/config.py b/nemo_rl/algorithms/single_controller_utils/config.py index a766e8675fb..f51e0c57e64 100644 --- a/nemo_rl/algorithms/single_controller_utils/config.py +++ b/nemo_rl/algorithms/single_controller_utils/config.py @@ -292,6 +292,9 @@ class TokenCaptureConfig(BaseModel, extra="allow"): # derived at setup # under the run's log dir. capture_dir: Optional[str] = None + # Generation backend hosting token capture. This is derived from + # policy.generation.backend during setup; users should not set it separately. + generation_backend: Optional[Literal["vllm", "megatron"]] = None # Keep routed_experts out of canonical rows and assemble them on policy # workers from strict staged-fragment plans. defer_routed_experts_to_policy: bool = False diff --git a/nemo_rl/algorithms/single_controller_utils/setup.py b/nemo_rl/algorithms/single_controller_utils/setup.py index 90ff8eae493..71061c444ca 100644 --- a/nemo_rl/algorithms/single_controller_utils/setup.py +++ b/nemo_rl/algorithms/single_controller_utils/setup.py @@ -32,6 +32,7 @@ from pathlib import Path from typing import Any, Callable, Optional, cast +import ray import torch from torchdata.stateful_dataloader import StatefulDataLoader from transformers import AutoProcessor @@ -99,6 +100,8 @@ from nemo_rl.models.generation.interfaces import ( resolve_routed_experts_dtype_name_for_model, ) +from nemo_rl.models.generation.megatron.config import MCoreGenerationConfig +from nemo_rl.models.generation.megatron.megatron_generation import MegatronGeneration from nemo_rl.models.generation.sglang.config import SGLangConfig from nemo_rl.models.generation.sglang.sglang_generation import SGLangGeneration from nemo_rl.models.generation.vllm import VllmGeneration @@ -389,10 +392,6 @@ def _build_clusters( return cluster, cluster # Non-colocated: split node into train + inference clusters. - assert backend != "megatron", ( - "The Megatron generation backend does not support non-colocated inference " - "in SingleController." - ) inference_resources = generation_config["colocated"]["resources"] inference_gpus_per_node = inference_resources["gpus_per_node"] if inference_gpus_per_node is None: @@ -516,6 +515,7 @@ def _build_trainer( *, weights_path: Optional[Path], optimizer_path: Optional[Path], + reserved_http_server_port: Optional[int] = None, ) -> tuple[Any, float]: """Build the TQ-mediated trainer (driver-side TQPolicy). @@ -526,6 +526,8 @@ def _build_trainer( processor: Optional AutoProcessor for VLM paths. weights_path: Checkpointed policy weights to resume from, or None. optimizer_path: Checkpointed optimizer state to resume from, or None. + reserved_http_server_port: Pre-published OpenAI server port for NeMo Gym; + set only when colocated Megatron generation serves from the trainer's rank 0. Returns: A tuple of (TQPolicy trainer, wall time spent in this call). @@ -543,11 +545,85 @@ def _build_trainer( init_optimizer=True, init_reference_model=init_reference_model, dp_cfg=master_config.data_plane, + reserved_http_server_port=reserved_http_server_port, ) return trainer, time.perf_counter() - t0 -def _spinup_gym(master_config: MasterConfig, base_urls: list[str]) -> tuple[Any, float]: +def _build_trainer_then_megatron_generation( + train_cluster: RayVirtualCluster, + master_config: MasterConfig, + tokenizer, + processor, + *, + inference_cluster: Optional[RayVirtualCluster], + weights_path: Optional[Path], + optimizer_path: Optional[Path], + reserved_http_server_port: Optional[int] = None, +) -> tuple[Any, Any, dict[str, float]]: + """Build the trainer, then Megatron generation, serially in that order. + + Colocated (`inference_cluster` None) wraps the trainer's policy (shared worker group). + Non-colocated builds a dedicated inference policy on `inference_cluster` with the weight + load skipped; the first weight sync transfers the real weights over the refit collective. + + Args: + train_cluster: Ray virtual cluster the trainer workers run on. + master_config: SC MasterConfig. + tokenizer: Tokenizer used by the policy. + processor: Optional AutoProcessor for VLM paths. + inference_cluster: Dedicated generation cluster for non-colocated, or None when colocated. + weights_path: Checkpointed policy weights to resume from, or None. + optimizer_path: Checkpointed optimizer state to resume from, or None. + reserved_http_server_port: Pre-published OpenAI server port for NeMo Gym. + colocated: routed to the trainer's policy (rank 0 lives with the trainer); + non-colocated: routed to the dedicated generation. + + Returns: + A tuple of (MegatronGeneration, TQPolicy trainer, per-phase wall + times keyed as "gen_time" and "trainer_time"). + """ + time_metrics = {} + + colocated = inference_cluster is None + # Rank 0 lives with the trainer when colocated, so the reserved port routes + # to whichever side serves: the trainer's policy or the dedicated engine. + trainer_port, gen_port = ( + (reserved_http_server_port, None) + if colocated + else (None, reserved_http_server_port) + ) + + trainer, time_metrics["trainer_time"] = _build_trainer( + train_cluster, + master_config, + tokenizer, + processor, + weights_path=weights_path, + optimizer_path=optimizer_path, + reserved_http_server_port=trainer_port, + ) + + t0 = time.perf_counter() + generation = MegatronGeneration( + config=master_config.policy, + tokenizer=tokenizer, + cluster=inference_cluster, + policy=trainer if colocated else None, + processor=processor, + weights_path=weights_path, + skip_weight_load=not colocated, + reserved_http_server_port=gen_port, + ) + time_metrics["gen_time"] = time.perf_counter() - t0 + + return generation, trainer, time_metrics + + +def _spinup_gym( + master_config: MasterConfig, + base_urls: list[str], +) -> tuple[Any, float]: """Spin up the NeMo-Gym actor against the reserved vLLM URLs. Args: @@ -623,6 +699,60 @@ def _maybe_inject_megatron_train_iters(master_config: MasterConfig) -> None: policy_config["megatron_cfg"]["train_iters"] = grpo_config.max_num_steps +def _maybe_apply_megatron_generation_overrides( + master_config: MasterConfig, *, use_nemo_gym: bool +) -> None: + """Validate and adapt the config for the Megatron generation backend.""" + policy_config = master_config.policy + generation_config = policy_config["generation"] + if generation_config["backend"] != "megatron": + return + + if not ( + "megatron_cfg" in policy_config and policy_config["megatron_cfg"]["enabled"] + ): + raise ValueError( + "policy.generation.backend='megatron' requires the Megatron trainer " + "(policy.megatron_cfg.enabled=true): refit transfers weights via Megatron's reshard; " + "colocated generation shares the training policy's worker group." + ) + + mcore_cfg = cast(MCoreGenerationConfig, generation_config)[ + "mcore_generation_config" + ] + if use_nemo_gym and not mcore_cfg["expose_http_server"]: + raise ValueError( + "NeMo Gym usage requires " + "policy.generation.mcore_generation_config.expose_http_server=true" + ) + + async_config = master_config.async_rl + if async_config.recompute_kv_cache_after_weight_updates: + # As in grpo.py, recompute-after-refit is expressed engine-side for Megatron. + # Unlike grpo.py, SC also clears the flag so the actor skips its loop-level + # invalidate_kv_cache (a base-class no-op for MegatronGeneration). + prior_mode = mcore_cfg.get("kv_cache_management_mode") + if prior_mode != "recompute": + print( + f"kv_cache_management_mode overridden '{prior_mode}' -> 'recompute' by " + f"async_rl.recompute_kv_cache_after_weight_updates=True." + ) + # pyrefly: ignore[typed-dict-key-error] + mcore_cfg["kv_cache_management_mode"] = "recompute" + async_config.recompute_kv_cache_after_weight_updates = False + + if generation_config["colocated"]["enabled"]: + num_prompts_per_step = master_config.grpo.num_prompts_per_step + if async_config.max_buffered_rollouts < num_prompts_per_step: + raise ValueError( + f"async_rl.max_buffered_rollouts " + f"({async_config.max_buffered_rollouts}) must be >= " + f"grpo.num_prompts_per_step ({num_prompts_per_step}) for " + "colocated megatron generation: the buffer must be able to " + "hold a full step before the trainer takes the GPUs." + ) + + def _build_retry_policy(master_config: MasterConfig) -> RolloutRetryPolicy: """Translate ``async_rl.rollout_failure`` into the rollout layer's policy object.""" failure_config = master_config.async_rl.rollout_failure @@ -636,6 +766,25 @@ def _build_retry_policy(master_config: MasterConfig) -> RolloutRetryPolicy: ) +def _raise_missing_nemo_gym_error(error: Exception, backend: str) -> None: + """Raise backend-specific remediation for a missing Gym capture extra.""" + if backend == "megatron": + raise RuntimeError( + "Megatron token capture requires nemo_gym in the driver environment. " + "Launch the driver with `uv run --extra nemo_gym ...` or run " + "`uv sync --extra nemo_gym` before rerunning." + ) from error + # Worker venvs are cached by actor class name (nemo_rl/utils/venvs.py), so a + # venv prebuilt before token capture predates the nemo_gym extra and is reused. + raise RuntimeError( + "vLLM token capture requires nemo_gym in the " + "VllmAsyncGenerationWorker environment, but the cached worker venv " + "predates it. Rebuild worker venvs (NRL_FORCE_REBUILD_VENVS=true) or " + "delete $NEMO_RL_VENV_DIR/nemo_rl.models.generation.vllm." + "vllm_worker_async.VllmAsyncGenerationWorker and rerun." + ) from error + + def setup_single_controller( master_config: MasterConfig, tokenizer: PreTrainedTokenizerBase, @@ -788,29 +937,45 @@ def setup_single_controller( "(env.should_use_nemo_gym=true) — the ledger lives in Gym's " "policy model server" ) - if generation_config["backend"] != "vllm": + if generation_config["backend"] not in ("vllm", "megatron"): raise NotImplementedError( - "token_capture.enabled supports the vllm backend only; got " + "token_capture.enabled supports vllm or megatron; got " f"{generation_config['backend']!r}" ) - if not generation_config["vllm_cfg"]["async_engine"]: + if ( + generation_config["backend"] == "vllm" + and not generation_config["vllm_cfg"]["async_engine"] + ): raise ValueError( "token_capture.enabled requires " "policy.generation.vllm_cfg.async_engine=true (the capture " "host is the worker's in-process HTTP server)" ) - from nemo_rl.distributed.ray_actor_environment_registry import ( - ACTOR_ENVIRONMENT_REGISTRY, - ) - from nemo_rl.distributed.virtual_cluster import PY_EXECUTABLES + if generation_config["backend"] == "megatron": + if not generation_config["mcore_generation_config"]["expose_http_server"]: + raise ValueError( + "Megatron token capture requires policy.generation." + "mcore_generation_config.expose_http_server=true" + ) + if router_replay_enabled(master_config.policy): + raise NotImplementedError( + "Megatron token capture does not yet support router replay: " + "MInf ledger routes are not delta-token aligned" + ) + else: + from nemo_rl.distributed.ray_actor_environment_registry import ( + ACTOR_ENVIRONMENT_REGISTRY, + ) + from nemo_rl.distributed.virtual_cluster import PY_EXECUTABLES - ACTOR_ENVIRONMENT_REGISTRY[ - "nemo_rl.models.generation.vllm.vllm_worker_async.VllmAsyncGenerationWorker" - ] = PY_EXECUTABLES.VLLM_GYM + ACTOR_ENVIRONMENT_REGISTRY[ + "nemo_rl.models.generation.vllm.vllm_worker_async.VllmAsyncGenerationWorker" + ] = PY_EXECUTABLES.VLLM_GYM # Fill the derived ledger-hosting fields (see TokenCaptureConfig): a - # per-run control-plane bearer token and the process-shared capture - # directory used by every Gym worker. + # per-run control-plane bearer token, the process-shared capture + # directory used by every Gym worker, and the capture-host backend. + token_capture_cfg.generation_backend = generation_config["backend"] if token_capture_cfg.control_auth_token is None: # Deferred import: only needed on the capture path. import secrets @@ -941,11 +1106,13 @@ def setup_single_controller( # ========================== # TODO: add validate dataset wiring. use_nemo_gym = _should_use_nemo_gym(cast(GrpoMasterConfig, master_config)) - if use_nemo_gym and generation_config["backend"] != "vllm": + if use_nemo_gym and generation_config["backend"] not in ("vllm", "megatron"): raise NotImplementedError( - "SC NeMo-Gym integration currently supports the vllm backend " - f"only; got {generation_config['backend']!r}" + "SC NeMo-Gym integration currently supports the vllm and megatron backends only; got " + f"{generation_config['backend']!r}" ) + # Megatron-generation checks are pure config: run them before the dataset download. + _maybe_apply_megatron_generation_overrides(master_config, use_nemo_gym=use_nemo_gym) if use_nemo_gym: # NeMo-Gym creates the env actor outside setup_response_data; we wire # it in after generation is up (it needs the OpenAI server URLs). @@ -996,6 +1163,13 @@ def setup_single_controller( generation = None defer_generation_model_load = False gen_reserve_time = 0.0 + megatron_backend = generation_config["backend"] == "megatron" + megatron_reserved_url = None + megatron_port_holder = None + reserved_http_server_port = None + if megatron_backend: + # Normally set inside _build_generation, which megatron skips. + generation_config["model_name"] = master_config.policy["model_name"] def _build_generation_then_trainer( defer_generation_model_load: bool, generation=None @@ -1036,21 +1210,55 @@ def _build_generation_then_trainer( return generation, trainer, time_metrics if use_nemo_gym: - # defer generation, only get base_urls for nemo_gym spinup - generation, gen_reserve_time = _build_generation( - inference_cluster, - master_config=master_config, - defer_model_load=True, - ) - defer_generation_model_load = True + if megatron_backend: + # Megatron serves from rank 0 of the generation workers; pre-publish that address. + t0 = time.perf_counter() + ( + megatron_reserved_url, + reserved_http_server_port, + megatron_port_holder, + ) = MegatronGeneration.reserve_http_server_address( + train_cluster if colocated else inference_cluster, + master_config.policy, + ) + gen_reserve_time = time.perf_counter() - t0 + print( + f" ✓ Reserved Megatron server URL: {megatron_reserved_url}", + flush=True, + ) + gym_base_urls: list[Optional[str]] = [megatron_reserved_url] + else: + # defer generation, only get base_urls for nemo_gym spinup + generation, gen_reserve_time = _build_generation( + inference_cluster, + master_config=master_config, + defer_model_load=True, + ) + defer_generation_model_load = True + gym_base_urls = generation.dp_openai_server_base_urls # add nemo_gym spinup task build_tasks["nemo_gym"] = partial( _spinup_gym, master_config=master_config, - base_urls=generation.dp_openai_server_base_urls, + base_urls=gym_base_urls, ) - if colocated: + if megatron_backend: + # Serial trainer-first in both modes: + # colocated generation is constructed from the trainer's policy; + # non-colocated waits for the trainer's checkpoint conversion. + build_tasks["generation_trainer"] = partial( + _build_trainer_then_megatron_generation, + train_cluster, + master_config, + tokenizer, + processor, + inference_cluster=None if colocated else inference_cluster, + weights_path=weights_path, + optimizer_path=optimizer_path, + reserved_http_server_port=reserved_http_server_port, + ) + elif colocated: # Colocated: vLLM prefers a clean GPU at load time, so generation comes up before the trainer. build_tasks["generation_trainer"] = partial( _build_generation_then_trainer, @@ -1081,11 +1289,16 @@ def _build_generation_then_trainer( ) # Submit build tasks and get results - with ThreadPoolExecutor(max_workers=len(build_tasks)) as executor: - submitted = {k: executor.submit(fn) for k, fn in build_tasks.items()} - results = {k: f.result() for k, f in submitted.items()} - - if colocated: + try: + with ThreadPoolExecutor(max_workers=len(build_tasks)) as executor: + submitted = {k: executor.submit(fn) for k, fn in build_tasks.items()} + results = {k: f.result() for k, f in submitted.items()} + finally: + if megatron_port_holder is not None: + # Rank 0 adopted (or will never adopt) the held socket; drop the holder. + ray.kill(megatron_port_holder) + + if "generation_trainer" in results: generation, trainer, time_metrics = results["generation_trainer"] gen_load_time = time_metrics["gen_time"] setup_timing_metrics.policy_init_time_s = time_metrics["trainer_time"] @@ -1129,6 +1342,11 @@ def _build_generation_then_trainer( setup_timing_metrics.generation_init_reserve_time_s = gen_reserve_time setup_timing_metrics.generation_init_load_time_s = gen_load_time + if megatron_reserved_url is not None: + MegatronGeneration.verify_served_address( + generation.dp_openai_server_base_urls, megatron_reserved_url + ) + worker_setup_time = time.perf_counter() - setup_start_time setup_timing_metrics.worker_setup_time_s = worker_setup_time @@ -1181,25 +1399,15 @@ def _build_generation_then_trainer( num_samples=num_rollout_samples, consumer_tasks=["finalize", "prev_lp", "train"], ) - # Host Gym's capture core in every vLLM DP leader (in-worker DP - # client + TQTokenSink + the single install_capture call), and give - # workers the initial weight version to stamp on captured calls. + # vLLM stages in its serving workers. MInf enables its local ledgers + # and installs one driver-side ledger-to-TQ converter. try: generation.setup_token_capture( dp_config, token_capture_cfg.staging_partition ) except Exception as error: if "No module named 'nemo_gym'" in str(error): - # Worker venvs are cached by actor class name - # (nemo_rl/utils/venvs.py), so a venv prebuilt before token - # capture predates the nemo_gym extra and is reused as-is. - raise RuntimeError( - "token_capture.enabled requires nemo_gym inside the vLLM " - "worker venv, but the cached worker venv predates it. " - "Rebuild worker venvs (NRL_FORCE_REBUILD_VENVS=true) or " - "delete $NEMO_RL_VENV_DIR/nemo_rl.models.generation.vllm." - "vllm_worker_async.VllmAsyncGenerationWorker and rerun." - ) from error + _raise_missing_nemo_gym_error(error, generation_config["backend"]) raise generation.set_rollout_weight_version(0) diff --git a/nemo_rl/distributed/held_port.py b/nemo_rl/distributed/held_port.py new file mode 100644 index 00000000000..510a40af714 --- /dev/null +++ b/nemo_rl/distributed/held_port.py @@ -0,0 +1,89 @@ +# Copyright (c) 2026, NVIDIA CORPORATION. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import socket +import threading + +import ray + +from nemo_rl.distributed.virtual_cluster import _get_node_ip_local + + +def _held_port_uds_name(port: int) -> str: + """Abstract-namespace Unix socket where a HeldPortReservation serves its fd.""" + return f"\0nemo_rl_held_port_{port}" + + +def receive_held_socket(port: int) -> socket.socket: + """Adopt the listening socket held by this node's HeldPortReservation. + + Args: + port: The reserved port; names the same-node handoff endpoint. + + Returns: + The live listening socket, duplicated into this process. + """ + client = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + try: + client.connect(_held_port_uds_name(port)) + _, fds, _, _ = socket.recv_fds(client, 1024, 1) + except OSError as e: + raise RuntimeError( + f"Could not receive the reserved server socket for port {port}: " + "the port holder on this node is gone, so the pre-published URL would be unreachable." + ) from e + finally: + client.close() + if not fds: + raise RuntimeError(f"Port holder for port {port} sent no file descriptor.") + return socket.socket(fileno=fds[0]) + + +class HeldPortReservation: + """Bind-and-hold port reservation with fd handoff to a same-node process. + + The listening socket stays open from reservation until the eventual server adopts it, + so there is zero gap in which the kernel could hand the port to anyone else. + """ + + def __init__(self) -> None: + self._sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + self._sock.bind(("", 0)) + self._sock.listen(128) + self._port = self._sock.getsockname()[1] + self._node_ip = _get_node_ip_local() + self._uds = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + self._uds.bind(_held_port_uds_name(self._port)) + self._uds.listen(1) + threading.Thread(target=self._serve_fd_once, daemon=True).start() + + def address(self) -> tuple[str, int]: + """Return (node_ip, held_port).""" + return self._node_ip, self._port + + def _serve_fd_once(self) -> None: + conn, _ = self._uds.accept() + try: + socket.send_fds(conn, [b"s"], [self._sock.fileno()]) + finally: + conn.close() + self._uds.close() + # The receiver holds a duplicate fd; the local one is done. + self._sock.close() + + +# Classes with @ray.remote can't be inherited from, so we split the implementation out. +# The caller pins this to the bundle rank 0 will occupy. +@ray.remote(num_cpus=0) # pragma: no cover +class RemoteHeldPortReservation(HeldPortReservation): + pass diff --git a/nemo_rl/environments/nemo_gym.py b/nemo_rl/environments/nemo_gym.py index 39b70e6d941..05905ea21dd 100644 --- a/nemo_rl/environments/nemo_gym.py +++ b/nemo_rl/environments/nemo_gym.py @@ -187,6 +187,19 @@ class NemoGymConfig(TypedDict): _UNCOMMITTED_CALL_REASON = "request_finished_without_staged_coordinates" +def _external_staging_backend(token_capture: Dict[str, Any]) -> str: + """Map the setup-derived generation backend to Gym's capture backend.""" + generation_backend = token_capture.get("generation_backend") + if generation_backend == "vllm": + return "vllm_worker" + if generation_backend == "megatron": + return "megatron_ledger" + raise ValueError( + "token_capture.enabled requires setup-derived generation_backend to be " + f"'vllm' or 'megatron'; got {generation_backend!r}" + ) + + def _detect_invalid_tool_call_and_malformed_thinking( output_item_dict: dict[str, Any], invalid_tool_call_patterns: list[str] | None = None, @@ -562,6 +575,7 @@ def _spinup(self) -> None: self._control_headers: Dict[str, str] = {} self._control_timeout_s = 60.0 if self._token_capture_enabled: + assert token_capture is not None if self.rollout_max_attempts_to_avoid_lp_nan != 1: raise ValueError( "token_capture.enabled requires " @@ -587,6 +601,7 @@ def _spinup(self) -> None: "lineage_store": ("nemo_gym.token_id_capture.lineage:FileLineageStore"), "lineage_store_kwargs": {"root": os.path.join(capture_dir, "lineage")}, "external_staging": True, + "external_staging_backend": _external_staging_backend(token_capture), "control_auth_token_env": _TOKEN_CAPTURE_CONTROL_ENV, } # Gym resolves the credential inside each serving process. Keep @@ -844,9 +859,17 @@ def _assemble_receipt( and poisons. """ records = [dict(record) for record in manifest.get("records") or []] + pending_records = [ + dict(record) for record in manifest.get("pending_records") or [] + ] + if records and pending_records: + raise ValueError( + "capture manifest cannot mix staged and pending call records" + ) + receipt_records = pending_records or records failures = list(manifest.get("failures") or []) deduped: dict[str, dict] = {} - for record in records: + for record in receipt_records: deduped.setdefault(str(record.get("model_call_id")), record) terminal_record = None selection_reason = None @@ -863,14 +886,18 @@ def _assemble_receipt( else: terminal_selection = "heuristic" # Deferred: nemo_gym is an optional extra absent in non-gym runs. - from nemo_gym.token_id_capture.staging.records import CallRecord + from nemo_gym.token_id_capture.staging.records import ( + CallRecord, + PendingCallRecord, + ) from nemo_gym.token_id_capture.staging.terminal import ( select_terminal_call, ) try: + record_type = PendingCallRecord if pending_records else CallRecord selection = select_terminal_call( - [CallRecord.model_validate(record) for record in deduped.values()] + [record_type.model_validate(record) for record in deduped.values()] ) except ValueError: selection_reason = "invalid_manifest_row" @@ -891,7 +918,7 @@ def _assemble_receipt( ) elif terminal_record is None: failure_reason = selection_reason or "missing_terminal_row" - return { + receipt = { "rollout_id": rollout_id, "reward": reward, "terminal_model_call_id": ( @@ -899,11 +926,14 @@ def _assemble_receipt( if terminal_record is not None else None ), - "manifest": list(deduped.values()), + "manifest": [] if pending_records else list(deduped.values()), "capture_poisoned": failure_reason is not None, "failure_reason": failure_reason, "terminal_selection": terminal_selection, } + if pending_records: + receipt["pending_manifest"] = list(deduped.values()) + return receipt def _postprocess_nemo_gym_to_nemo_rl_result( self, @@ -1272,9 +1302,18 @@ def validate_reward_components_match_scalar(nemo_gym_results: List[dict]) -> Non def setup_nemo_gym_config(config, tokenizer) -> None: generation_config = config.policy["generation"] - # Enable the http server. Requires both async engine and the expose_http_server flag - generation_config["vllm_cfg"]["async_engine"] = True - generation_config["vllm_cfg"]["expose_http_server"] = True + # Enable the backend's OpenAI-compatible server. + if generation_config["backend"] == "vllm": + generation_config["vllm_cfg"]["async_engine"] = True + generation_config["vllm_cfg"]["expose_http_server"] = True + elif generation_config["backend"] == "megatron": + generation_config["mcore_generation_config"]["async_engine"] = True + generation_config["mcore_generation_config"]["expose_http_server"] = True + else: + raise ValueError( + "NeMo-Gym setup supports vllm or megatron generation; got " + f"{generation_config['backend']!r}" + ) # Stop strings or token ids are not supported generation_config["stop_strings"] = None diff --git a/nemo_rl/experience/rollout_manager.py b/nemo_rl/experience/rollout_manager.py index 4c178092cc5..ab87ead1f1c 100644 --- a/nemo_rl/experience/rollout_manager.py +++ b/nemo_rl/experience/rollout_manager.py @@ -1274,6 +1274,7 @@ def __init__( stats=self._stats, ) self._tokenizer = tokenizer + self._policy_generation = policy_generation self._num_generations_per_prompt = num_generations_per_prompt self._tq_buffer = tq_buffer self._recovery_ledger = recovery_ledger @@ -1775,24 +1776,36 @@ async def _record_streamed_completion( raise ValueError( "token-capture completion must contain its Gate rollout ID" ) - barrier = self._data_plane_checkpoint_barrier - if barrier is None: + typed_receipt: dict[str, Any] = receipt + typed_gate_rollout_id: str = gate_rollout_id + + async def _flush_and_seal() -> None: + if typed_receipt.get("pending_manifest"): + if self._policy_generation is None: + raise RuntimeError( + "deferred token capture requires a generation backend" + ) + finalized = await asyncio.to_thread( + self._policy_generation.flush_token_capture, typed_receipt + ) + # The completion and recovery ledger retain this mapping. + # Mutate it only after the whole ledger batch is durable. + typed_receipt.clear() + typed_receipt.update(finalized) self._recovery_ledger.mark_sibling_sealed( group_id, generation_index=generation_index, - gate_rollout_id=gate_rollout_id, - receipt=receipt, + gate_rollout_id=typed_gate_rollout_id, + receipt=typed_receipt, reward=completion.reward, ) + + barrier = self._data_plane_checkpoint_barrier + if barrier is None: + await _flush_and_seal() else: async with barrier.mutation("sibling_seals"): - self._recovery_ledger.mark_sibling_sealed( - group_id, - generation_index=generation_index, - gate_rollout_id=gate_rollout_id, - receipt=receipt, - reward=completion.reward, - ) + await _flush_and_seal() try: if inflight_registry is not None: diff --git a/nemo_rl/models/generation/interfaces.py b/nemo_rl/models/generation/interfaces.py index 6e8e539b979..cc7101ad9fd 100644 --- a/nemo_rl/models/generation/interfaces.py +++ b/nemo_rl/models/generation/interfaces.py @@ -399,6 +399,22 @@ def nccl_reshard_refit(self) -> list[ray.ObjectRef]: def invalidate_kv_cache(self) -> bool: return False + def flush_token_capture(self, receipt: dict[str, Any]) -> dict[str, Any]: + """Make a deferred capture receipt durable before it is sealed.""" + if receipt.get("pending_manifest"): + raise NotImplementedError( + f"{type(self).__name__} does not implement deferred token capture" + ) + return receipt + + def blocks_training(self) -> bool: + """Whether this engine must stand down before a training step.""" + return False + + def wake_carries_weight_updates(self) -> bool: + """Whether waking this engine alone serves weights updated while asleep.""" + return False + def clear_logger_metrics(self) -> None: """Clear logger metrics for performance reporting. diff --git a/nemo_rl/models/generation/megatron/config.py b/nemo_rl/models/generation/megatron/config.py index 94f9972c0d5..db8e2805722 100644 --- a/nemo_rl/models/generation/megatron/config.py +++ b/nemo_rl/models/generation/megatron/config.py @@ -28,7 +28,6 @@ class MCoreGenerationSpecificArgs(TypedDict): async_engine: bool expose_http_server: bool parsers: list[str] - buffer_size_gb: int block_size_tokens: int max_tokens: int diff --git a/nemo_rl/models/generation/megatron/megatron_generation.py b/nemo_rl/models/generation/megatron/megatron_generation.py index 866f3c6b8f6..21d3e2ac828 100644 --- a/nemo_rl/models/generation/megatron/megatron_generation.py +++ b/nemo_rl/models/generation/megatron/megatron_generation.py @@ -12,13 +12,17 @@ # See the License for the specific language governing permissions and # limitations under the License. +import logging +import threading from typing import TYPE_CHECKING, Any, AsyncGenerator, Optional import ray +from ray.util.scheduling_strategies import PlacementGroupSchedulingStrategy from transformers import AutoProcessor from transformers.tokenization_utils_base import PreTrainedTokenizerBase from nemo_rl.distributed.batched_data_dict import BatchedDataDict +from nemo_rl.distributed.held_port import RemoteHeldPortReservation from nemo_rl.distributed.virtual_cluster import RayVirtualCluster from nemo_rl.models.generation.interfaces import ( GenerationDatumSpec, @@ -29,9 +33,14 @@ from nemo_rl.models.policy import PolicyConfig if TYPE_CHECKING: + from nemo_rl.data_plane.interfaces import DataPlaneConfig + from nemo_rl.distributed.worker_groups import RayWorkerGroup from nemo_rl.models.policy.lm_policy import Policy +LOGGER = logging.getLogger(__name__) + + class MegatronGeneration(GenerationInterface): """Generation interface backed by Megatron (colocated or non-colocated).""" @@ -80,6 +89,64 @@ def init_cluster_placement_groups( use_unified_pg=cls.nvlink_domain_span(config) > cluster.num_gpus_per_node, ) + @classmethod + def reserve_http_server_address( + cls, + cluster: RayVirtualCluster, + config: PolicyConfig, + ) -> tuple[str, int, ray.actor.ActorHandle]: + """Reserve the OpenAI server address before any generation worker exists. + + This is megatron's substitute for vLLM's `defer_model_load` overlap. + See https://github.com/NVIDIA-NeMo/RL/issues/3752 + + Args: + cluster: The cluster the generation workers will run on. + config: The full `PolicyConfig`. + + Returns: + Tuple of (server base URL, reserved port, port-holder actor handle). + The caller must keep the handle referenced until rank 0 has adopted + the socket (worker init complete), then `ray.kill` it. + """ + # Colocated generation shares the training policy's cluster and uses the + # default placement-group init, triggered lazily by the read below. + if not config["generation"]["colocated"]["enabled"]: + cls.init_cluster_placement_groups(cluster, config) + + # Distributed rank 0 lands on the first bundle handed to the worker + # group: sorted-first for a unified placement group, else bundle 0 of + # the first group (mirrors Policy's worker-group construction). + placement_groups = cluster.get_placement_groups() + rank0_bundle_index = ( + cluster._sorted_bundle_indices[0] + if cluster._sorted_bundle_indices is not None + else 0 + ) + # Zero-gap reservation: a holder actor on the rank-0 node binds and + # HOLDS the socket (num_cpus=0, so it schedules even on a full bundle); + # rank 0 later adopts the live fd via receive_held_socket, so the port + # can never be stolen in between and any free port is safe. + holder = RemoteHeldPortReservation.options( + scheduling_strategy=PlacementGroupSchedulingStrategy( + placement_group=placement_groups[0], + placement_group_bundle_index=rank0_bundle_index, + ), + ).remote() + node_ip, port = ray.get(holder.address.remote()) + return f"http://{node_ip}:{port}/v1", port, holder + + @classmethod + def verify_served_address( + cls, served_urls: list[Optional[str]], reserved_url: str + ) -> None: + """Fail loud if the engine serves anywhere but the pre-published address.""" + if served_urls != [reserved_url]: + raise RuntimeError( + "Megatron server came up at a different address than the one " + f"pre-published to NeMo Gym: reserved {reserved_url}, serving {served_urls}." + ) + def __init__( self, config: PolicyConfig, @@ -90,6 +157,7 @@ def __init__( processor: Optional[AutoProcessor] = None, weights_path: Optional[str] = None, skip_weight_load: bool = False, + reserved_http_server_port: Optional[int] = None, ): """Initialize a MegatronGeneration instance. @@ -104,6 +172,7 @@ def __init__( processor: Optional processor for VLMs (non-colocated only). weights_path: Optional path to model weights (non-colocated only). skip_weight_load: Do not load the weights from the checkpoint; refit will do it. + reserved_http_server_port: Driver-reserved OpenAI server port for non-colocated. """ # Import here to avoid circular imports from nemo_rl.models.policy.lm_policy import Policy @@ -114,6 +183,10 @@ def __init__( assert not (skip_weight_load and policy is not None), ( "skip_weight_load only applies to the dedicated inference policy." ) + assert not (reserved_http_server_port is not None and policy is not None), ( + "reserved_http_server_port only applies to the dedicated inference " + "policy; when colocated, pass it to the training policy instead." + ) # `self.cfg` exposes the `generation` that matches the `GenerationInterface` contract. # `self._policy_config` keeps a reference to the full PolicyConfig. @@ -121,6 +194,8 @@ def __init__( self.cfg: MCoreGenerationConfig = config["generation"] # Populated after the first prepare_for_generation (which starts the HTTP server). self.dp_openai_server_base_urls: list[Optional[str]] = [] + self._token_capture = None + self._token_capture_flush_lock = threading.Lock() if policy is not None: # Reuse the existing training policy. @@ -151,11 +226,17 @@ def __init__( init_reference_model=False, weights_path=weights_path, skip_weight_load=skip_weight_load, + reserved_http_server_port=reserved_http_server_port, ) # Start the persistent inference engine + HTTP server during construction. self.prepare_for_generation() + @property + def worker_group(self) -> "RayWorkerGroup": + """The underlying policy's worker group (fleet-health probes read dp_size).""" + return self._policy.worker_group + def init_collective( self, ip: str, @@ -256,6 +337,209 @@ def finish_generation(self, *args: Any, **kwargs: Any) -> bool: ray.get(futures) return True + def setup_token_capture( + self, dp_cfg: "DataPlaneConfig", staging_partition: str + ) -> None: + """Enable each MInf ledger and install the driver-side TQ converter.""" + if not self.cfg["mcore_generation_config"]["expose_http_server"]: + raise ValueError( + "Megatron token capture requires mcore_generation_config." + "expose_http_server=true" + ) + # Deferred: nemo_gym is an optional extra absent in non-Gym runs. + from nemo_gym.token_id_capture.adapters.megatron import MegatronCaptureAdapter + from nemo_gym.token_id_capture.staging.capture import RolloutTokenCapture + + from nemo_rl.data_plane import build_data_plane_client + from nemo_rl.data_plane.tq_token_sink import TQTokenSink + + dp_client = build_data_plane_client(dp_cfg, bootstrap=False) + sink = TQTokenSink(dp_client, staging_partition=staging_partition) + self._token_capture = RolloutTokenCapture( + sink=sink, + weight_version_fn=lambda: 0, + adapter=MegatronCaptureAdapter(), + ) + futures = self._policy.worker_group.run_all_workers_single_data( + "setup_token_capture" + ) + ray.get(futures) + + def set_rollout_weight_version(self, version: int) -> None: + """Rotate the policy epoch stamped by MInf on subsequent requests.""" + futures = self._policy.worker_group.run_all_workers_single_data( + "set_rollout_weight_version", version=version + ) + ray.get(futures) + + def flush_token_capture(self, receipt: dict[str, Any]) -> dict[str, Any]: + """Batch-convert one rollout's MInf ledger rows into durable TQ rows.""" + from nemo_gym.token_id_capture.staging.digest import compute_chain_hash + from nemo_gym.token_id_capture.staging.protocols import DeferredCaptureAdapter + from nemo_gym.token_id_capture.staging.records import ( + CaptureAdmission, + CallRecord, + PendingCallRecord, + RolloutReceipt, + ) + + capture = self._token_capture + if capture is None: + raise RuntimeError("Megatron token capture is not initialized") + pending_payloads = receipt.get("pending_manifest") + if not pending_payloads: + return receipt + if not isinstance(pending_payloads, list): + raise ValueError("pending_manifest must be a list") + if receipt.get("manifest"): + raise ValueError("a deferred receipt cannot already contain staged records") + + with self._token_capture_flush_lock: + pending = [ + PendingCallRecord.model_validate(payload) + for payload in pending_payloads + ] + request_uids = [record.ledger_request_uid for record in pending] + if len(request_uids) != len(set(request_uids)): + raise ValueError( + "pending_manifest contains duplicate MInf request UIDs" + ) + futures = self._policy.worker_group.run_all_workers_single_data( + "fetch_token_capture_records", request_uids=request_uids + ) + worker_records = ray.get(futures) + records_by_uid: dict[str, dict[str, Any]] = {} + for uid in request_uids: + matches = [records[uid] for records in worker_records if uid in records] + if len(matches) != 1: + raise RuntimeError( + f"MInf request {uid!r} was present on {len(matches)} engine ranks; expected 1" + ) + records_by_uid[uid] = matches[0] + + committed_records: list[CallRecord] = [] + adapter = capture.adapter + if not isinstance(adapter, DeferredCaptureAdapter): + raise RuntimeError("a deferred token capture adapter is not installed") + pending_by_call_id = {record.model_call_id: record for record in pending} + prepared_calls = [] + for record in pending: + ledger_payload = records_by_uid[record.ledger_request_uid] + prompt_token_ids = adapter.extract_prompt_ids(ledger_payload) + generated_token_ids, _ = adapter.extract_generation(ledger_payload) + if record.prev_len > len(prompt_token_ids): + raise RuntimeError( + f"MInf request {record.ledger_request_uid!r} has a prompt " + f"shorter than prev_len={record.prev_len}" + ) + actual_delta_len = ( + len(prompt_token_ids) - record.prev_len + len(generated_token_ids) + ) + if ( + actual_delta_len != record.delta_len + or record.prev_len + actual_delta_len != record.cum_len + ): + raise RuntimeError( + f"MInf request {record.ledger_request_uid!r} differs from " + "the HTTP lineage lengths" + ) + parent_chain_hash = None + if record.parent_call_id is not None: + parent = pending_by_call_id.get(record.parent_call_id) + if parent is None: + raise RuntimeError( + f"MInf call {record.model_call_id!r} references missing " + f"parent {record.parent_call_id!r}" + ) + parent_chain_hash = parent.chain_hash + ledger_delta = prompt_token_ids[record.prev_len :] + generated_token_ids + if ( + compute_chain_hash(parent_chain_hash, ledger_delta) + != record.chain_hash + ): + raise RuntimeError( + f"MInf request {record.ledger_request_uid!r} token content " + "differs from the HTTP lineage" + ) + admission = CaptureAdmission( + rollout_id=str(receipt["rollout_id"]), + model_call_id=record.model_call_id, + parent_call_id=record.parent_call_id, + prev_len=record.prev_len, + mode=record.mode, + required_prefix_token_ids=( + prompt_token_ids[: record.prev_len] + if record.mode == "token_in" + else [] + ), + parent_chain_hash=parent_chain_hash, + ) + weight_version = adapter.extract_weight_version(ledger_payload) + prepared_calls.append( + (record, ledger_payload, admission, weight_version) + ) + + for record, ledger_payload, admission, weight_version in prepared_calls: + call = capture.begin_call(admission, weight_version=weight_version) + coords = capture.complete_call_from_response(call, ledger_payload) + if coords.disposition != "staged": + raise RuntimeError( + f"failed to stage MInf request {record.ledger_request_uid!r}" + ) + committed_records.append( + CallRecord( + model_call_id=coords.model_call_id, + parent_call_id=coords.parent_call_id, + prev_len=coords.prev_len, + delta_len=coords.delta_len, + cum_len=coords.cum_len, + weight_version=coords.weight_version, + digest=coords.digest, + extras_digest=coords.extras_digest, + staging_key=coords.staging_key, + mode=record.mode, + chain_hash=coords.chain_hash, + cumulative_hash=coords.cumulative_hash, + response_id=record.response_id, + logical_request_id=record.logical_request_id, + admitted_at=record.admitted_at, + ) + ) + + finalized = dict(receipt) + finalized.pop("pending_manifest", None) + finalized["manifest"] = [ + record.model_dump(mode="json") for record in committed_records + ] + RolloutReceipt.model_validate(finalized) + + discard_futures = self._policy.worker_group.run_all_workers_single_data( + "discard_token_capture_records", request_uids=request_uids + ) + try: + discarded = ray.get(discard_futures) + if sum(int(count) for count in discarded) != len(request_uids): + raise RuntimeError( + "MInf ledger discard count did not match the staged batch" + ) + except Exception: + # TQ is now authoritative. Retaining a duplicate in the + # ephemeral ledger is safe and preferable to unsealing a + # rollout whose durable rows already exist. + LOGGER.warning( + "Could not discard MInf ledger rows after durable staging", + exc_info=True, + ) + return finalized + + def blocks_training(self) -> bool: + """Whether colocated generation must stand down before training.""" + return bool(self.cfg["colocated"]["enabled"]) + + def wake_carries_weight_updates(self) -> bool: + """Colocated wake reshards or shares the updated training tensors.""" + return bool(self.cfg["colocated"]["enabled"]) + def preinit_nvshmem_collective(self) -> list[ray.ObjectRef]: """Pre-initialize NVShmem collectively after CUDA graph capture. diff --git a/nemo_rl/models/generation/megatron/megatron_worker.py b/nemo_rl/models/generation/megatron/megatron_worker.py index d194e7d77b7..dc1c0b9616b 100644 --- a/nemo_rl/models/generation/megatron/megatron_worker.py +++ b/nemo_rl/models/generation/megatron/megatron_worker.py @@ -57,6 +57,7 @@ class MegatronGenerationMixin: - tokenizer: HF tokenizer. - megatron_tokenizer: tokenizer for inference. - is_generation_colocated: Whether colocated or distributed. + - _reserved_http_server_socket: driver-reserved server socket, or None. """ def _init_inference_engine_state(self) -> None: @@ -72,6 +73,7 @@ def _init_inference_engine_state(self) -> None: ) self._inference_loop = None self._inference_thread = None + self._token_capture_enabled = False def _initialize_inference_engine(self, mcore_generation_config: dict) -> None: """Initialize the persistent inference engine and client.""" @@ -288,18 +290,23 @@ def _setup_openai_api_server(self) -> str: ) ip = _get_node_ip_local() - free_port = _get_free_port_local() + reserved_socket = self._reserved_http_server_socket + if reserved_socket is not None: + server_port = reserved_socket.getsockname()[1] + else: + server_port = _get_free_port_local() start_text_gen_server( coordinator_addr=self.coordinator_addr, tokenizer=self.megatron_tokenizer, rank=torch.distributed.get_rank(), - server_port=free_port, + server_port=server_port, parsers=self.cfg["generation"]["mcore_generation_config"]["parsers"], verbose=False, + sock=reserved_socket, ) - base_url = f"http://{ip}:{free_port}/v1" + base_url = f"http://{ip}:{server_port}/v1" max_wait_time = 300 start_time = time.time() with requests.Session() as session: @@ -424,6 +431,88 @@ def report_dp_openai_server_base_url(self) -> Optional[str]: """Return this worker's OpenAI server base URL (None if not the leader).""" return self.base_url + def setup_token_capture(self) -> bool: + """Enable MInf's local offload ledger on this engine rank.""" + engine = self.dynamic_inference_engine + if engine is None: + raise RuntimeError( + "Megatron token capture requires an initialized inference engine" + ) + missing = [ + name + for name in ( + "local_metadata_ledger_enabled", + "local_metadata_ledger_offload_enabled", + "fetch_from_metadata_ledger", + ) + if not hasattr(engine, name) + ] + if missing: + raise RuntimeError( + "Megatron token capture requires the MInf ledger-capture API; " + f"missing {', '.join(missing)}" + ) + engine.local_metadata_ledger_enabled = True + engine.local_metadata_ledger_offload_enabled = True + self._token_capture_enabled = True + return True + + def set_rollout_weight_version(self, version: int) -> None: + """Stamp subsequent MInf requests with the trainer weight version.""" + if type(version) is not int or version < 0: + raise ValueError( + f"rollout weight version must be a non-negative int, got {version!r}" + ) + if torch.distributed.get_rank() != 0: + return + if not self._token_capture_enabled or self.inference_client is None: + raise RuntimeError("Megatron token capture is not initialized") + setter = getattr(self.inference_client, "set_generation_epoch", None) + if not callable(setter): + raise RuntimeError( + "Megatron token capture requires InferenceClient.set_generation_epoch" + ) + setter(version) + + @staticmethod + def _serialize_token_capture_record(record) -> dict: + """Convert an MInf ``FinishedRequestRecord`` into Ray-safe plain data.""" + routes = record.routing_indices + if routes is not None: + routes = routes.tolist() if hasattr(routes, "tolist") else list(routes) + return { + "policy_epoch": record.policy_epoch, + "kv_cache_epoch": record.kv_cache_epoch, + "num_evictions": int(record.num_evictions), + "prompt_token_ids": record.prompt_token_ids, + "generated_token_ids": record.generated_token_ids, + "generated_log_probs": record.generated_log_probs, + "prompt_log_probs": record.prompt_log_probs, + "routing_indices": routes, + } + + def fetch_token_capture_records(self, request_uids: list[str]) -> dict[str, dict]: + """Read requested MInf ledger rows without relinquishing custody.""" + if not self._token_capture_enabled or self.dynamic_inference_engine is None: + raise RuntimeError("Megatron token capture is not initialized") + found = self.dynamic_inference_engine.fetch_from_metadata_ledger( + request_uids, pop=False + ) + return { + uid: self._serialize_token_capture_record(record) + for uid, record in found.items() + } + + def discard_token_capture_records(self, request_uids: list[str]) -> int: + """Delete ledger rows after their whole rollout is durable in TQ.""" + if not self._token_capture_enabled or self.dynamic_inference_engine is None: + raise RuntimeError("Megatron token capture is not initialized") + return len( + self.dynamic_inference_engine.fetch_from_metadata_ledger( + request_uids, pop=True + ) + ) + def _build_sampling_params( self, greedy: bool, stop_words: Optional[list[str]] ) -> SamplingParams: diff --git a/nemo_rl/models/policy/lm_policy.py b/nemo_rl/models/policy/lm_policy.py index 2108bb10e92..39b47e7bd74 100644 --- a/nemo_rl/models/policy/lm_policy.py +++ b/nemo_rl/models/policy/lm_policy.py @@ -102,6 +102,7 @@ def __init__( processor: Optional[AutoProcessor] = None, worker_extension_cls_fqn: Optional[str] = None, skip_weight_load: bool = False, + reserved_http_server_port: Optional[int] = None, ): self.debug_payload_metrics = False if weights_path: @@ -123,6 +124,11 @@ def __init__( "Configure either Megatron (policy.megatron_cfg.enabled=true) or " "DTensor (policy.dtensor_cfg.enabled=true), not both." ) + if reserved_http_server_port is not None and not megatron_enable: + raise ValueError( + "reserved_http_server_port is only supported by the Megatron " + "worker (policy.megatron_cfg.enabled=true)." + ) if draft_enabled and not megatron_enable: raise ValueError( "policy.draft.enabled=true is only supported with the Megatron backend. " @@ -266,6 +272,8 @@ def __init__( ) if skip_weight_load: worker_kwargs["skip_weight_load"] = True + if reserved_http_server_port is not None: + worker_kwargs["reserved_http_server_port"] = reserved_http_server_port if use_v2: # DTensor v2 workers reconstruct tokenizer/processor locally to avoid diff --git a/nemo_rl/models/policy/workers/megatron_policy_worker.py b/nemo_rl/models/policy/workers/megatron_policy_worker.py index 2c1e6882442..1a07d4ae49e 100644 --- a/nemo_rl/models/policy/workers/megatron_policy_worker.py +++ b/nemo_rl/models/policy/workers/megatron_policy_worker.py @@ -16,6 +16,7 @@ import logging import os import re +import socket import time import warnings from collections import OrderedDict, defaultdict @@ -52,6 +53,7 @@ from nemo_rl.algorithms.loss.interfaces import LossFunction from nemo_rl.data_plane.worker_mixin import TQWorkerMixin from nemo_rl.distributed.batched_data_dict import BatchedDataDict +from nemo_rl.distributed.held_port import receive_held_socket from nemo_rl.distributed.named_sharding import NamedSharding from nemo_rl.models.generation.interfaces import GenerationDatumSpec from nemo_rl.models.generation.megatron.megatron_worker import ( @@ -377,6 +379,7 @@ def __init__( *, worker_sharding_annotations: NamedSharding, skip_weight_load: bool = False, + reserved_http_server_port: Optional[int] = None, **kwargs: Any, ): """Initialize the MegatronPolicyWorker.""" @@ -412,6 +415,15 @@ def __init__( self.rank = get_rank_safe() self.timer = Timer(context={"worker": "megatron_policy", "rank": self.rank}) + # Adopt the driver-reserved OpenAI server socket before any heavy init. + # The port holder has kept it bound and listening since reservation, so + # there was no window in which the pre-published URL could be stolen. + self._reserved_http_server_socket: Optional[socket.socket] = None + if reserved_http_server_port is not None and self.rank == 0: + self._reserved_http_server_socket = receive_held_socket( + reserved_http_server_port + ) + # Step 1: Setup distributed setup_distributed() log_gpu_memory_diagnostics( diff --git a/pyrefly.toml b/pyrefly.toml index b5d32bc0006..ad277828999 100644 --- a/pyrefly.toml +++ b/pyrefly.toml @@ -130,6 +130,7 @@ project-includes = [ "nemo_rl/data_plane/worker_mixin.py", "nemo_rl/distributed/__init__.py", "nemo_rl/distributed/collectives.py", + "nemo_rl/distributed/held_port.py", "nemo_rl/distributed/named_sharding.py", "nemo_rl/distributed/numa_utils.py", "nemo_rl/distributed/ray_actor_environment_registry.py", diff --git a/tests/functional/L1_Functional_Tests_SingleController.sh b/tests/functional/L1_Functional_Tests_SingleController.sh index 5ffdf42244a..6944589fd3c 100755 --- a/tests/functional/L1_Functional_Tests_SingleController.sh +++ b/tests/functional/L1_Functional_Tests_SingleController.sh @@ -36,6 +36,9 @@ run_test() { run_test fast uv run --no-sync bash ./tests/functional/grpo_dp_single_controller.sh run_test fast uv run --no-sync bash ./tests/functional/grpo_async_gym_single_controller.sh +run_test fast uv run --no-sync bash ./tests/functional/grpo_megatron_generation_gym_single_controller.sh +# Full mode only: colocated reshard megatron. +run_test uv run --no-sync bash ./tests/functional/grpo_megatron_generation_colocated_reshard_gym_single_controller.sh # Full mode only (~10 min): SIGKILLs a generation worker and asserts the job fails fast # and attributably instead of wedging. This is the ONLY end-to-end check of the # containment behaviour -- without it, a regression that restores the silent wedge is diff --git a/tests/functional/grpo_megatron_generation_colocated_reshard_gym_single_controller.sh b/tests/functional/grpo_megatron_generation_colocated_reshard_gym_single_controller.sh new file mode 100755 index 00000000000..f5c15df0e29 --- /dev/null +++ b/tests/functional/grpo_megatron_generation_colocated_reshard_gym_single_controller.sh @@ -0,0 +1,123 @@ +#!/bin/bash +# SingleController + NeMo-Gym + colocated Megatron generation, reshard mode: +# TE TP2 training shares both GPUs with inference_optimized TP1 generation on +# a dedicated model, resharded into on every post-step wake. The SC pump +# sleeps the engine for each train step (whole-step phases); Gym spinup +# overlaps the trainer + engine init via the held-socket reservation. + +SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd) +PROJECT_ROOT=$(realpath $SCRIPT_DIR/../..) +# Mark the current repo as safe, since wandb fetches metadata about the repo +git config --global --add safe.directory $PROJECT_ROOT + +set -eou pipefail + +EXP_NAME=$(basename $0 .sh) +EXP_DIR=$SCRIPT_DIR/$EXP_NAME +LOG_DIR=$EXP_DIR/logs +JSON_METRICS=$EXP_DIR/metrics.json +RUN_LOG=$EXP_DIR/run.log +CHECKPOINT_DIR=$EXP_DIR/checkpoints +DATA_DIR=$EXP_DIR/data +export PYTHONPATH=${PROJECT_ROOT}:${PYTHONPATH:-} + +rm -rf $EXP_DIR $LOG_DIR +mkdir -p $EXP_DIR $LOG_DIR $CHECKPOINT_DIR $DATA_DIR + +# clean up checkpoint directory on exit +trap "rm -rf $CHECKPOINT_DIR" EXIT + +cd $PROJECT_ROOT + +# Follow nemo-gym instructions here to get this data: +# https://docs.nvidia.com/nemo/gym/0.1.0/tutorials/nemo-rl-grpo/setup.html#training-nemo-rl-grpo-setup +cd 3rdparty/Gym-workspace/Gym + +# We need HF_TOKEN to download the data from huggingface +if [[ ! -f env.yaml ]]; then + if [[ -z "${HF_TOKEN:-}" ]]; then + echo "[ERROR] HF_TOKEN is not set" + exit 1 + fi + echo "hf_token: $HF_TOKEN" >> env.yaml +fi + +uv run ng_prepare_data "+config_paths=[resources_servers/workplace_assistant/configs/workplace_assistant.yaml]" \ + +output_dirpath=data/workplace_assistant \ + +mode=train_preparation \ + +should_download=true \ + +data_source=huggingface +cd - + +# This trimming of the workplace assistant dataset is necessary b/c with all the tools the first prompt is >4000 tokens +# which will cause vllm to return nothing on the first prompt and crash RL. Since we want to keep this test short to +# smoke test, we trim all but the first tool +TRAIN_PATH=$DATA_DIR/workplace_assistant_train.jsonl +VALIDATION_PATH=$DATA_DIR/workplace_assistant_validation.jsonl +jq -c '.responses_create_params.tools |= (.[0:1])' 3rdparty/Gym-workspace/Gym/data/workplace_assistant/train.jsonl > $TRAIN_PATH +jq -c '.responses_create_params.tools |= (.[0:1])' 3rdparty/Gym-workspace/Gym/data/workplace_assistant/validation.jsonl > $VALIDATION_PATH + +uv run coverage run -a --data-file=$PROJECT_ROOT/tests/.coverage --source=$PROJECT_ROOT/nemo_rl \ + $PROJECT_ROOT/examples/run_grpo_single_controller.py \ + --config $PROJECT_ROOT/examples/nemo_gym/grpo_qwen3_30ba3b_instruct.yaml \ + policy.model_name=Qwen/Qwen3-0.6B \ + policy.dtensor_cfg.enabled=false \ + policy.megatron_cfg.enabled=true \ + policy.megatron_cfg.tensor_model_parallel_size=2 \ + policy.megatron_cfg.pipeline_model_parallel_size=1 \ + policy.megatron_cfg.expert_model_parallel_size=1 \ + policy.megatron_cfg.context_parallel_size=1 \ + policy.megatron_cfg.sequence_parallel=false \ + policy.generation.backend=megatron \ + policy.generation.mcore_generation_config.expose_http_server=true \ + ++policy.generation.mcore_generation_config.transformer_impl=inference_optimized \ + ++policy.generation.mcore_generation_config.tensor_model_parallel_size=1 \ + policy.generation.max_new_tokens=128 \ + policy.max_total_sequence_length=512 \ + policy.generation.colocated.enabled=true \ + grpo.num_prompts_per_step=4 \ + grpo.num_generations_per_prompt=2 \ + grpo.max_num_steps=10 \ + grpo.val_period=-1 \ + grpo.val_at_start=false \ + grpo.async_grpo=null \ + policy.train_global_batch_size=8 \ + policy.train_micro_batch_size=1 \ + cluster.gpus_per_node=2 \ + loss_fn.reference_policy_kl_penalty=0.01 \ + grpo.skip_reference_policy_logprobs_calculation=false \ + loss_fn.use_importance_sampling_correction=true \ + logger.tensorboard_enabled=true \ + logger.log_dir=$LOG_DIR \ + logger.wandb_enabled=false \ + logger.monitor_gpus=true \ + checkpointing.enabled=false \ + data.train.data_path=$TRAIN_PATH \ + data.validation.data_path=$VALIDATION_PATH \ + ++data_plane.enabled=true \ + ++data_plane.impl=transfer_queue \ + ++data_plane.backend=simple \ + ++data_plane.storage_capacity=1000000 \ + ++data_plane.num_storage_units=2 \ + ++data_plane.claim_meta_poll_interval_s=0.5 \ + ++data_plane.global_segment_size=549755813888 \ + ++data_plane.local_buffer_size=68719476736 \ + ++async_rl.sampler.name=in_order \ + ++async_rl.sampler.max_lookahead_versions=0 \ + ++async_rl.min_groups_for_streaming_train=4 \ + ++async_rl.max_inflight_prompts=4 \ + ++async_rl.max_buffered_rollouts=4 \ + $@ \ + 2>&1 | tee $RUN_LOG + +if ! grep -q "\[colocated-reshard\] building dedicated inference model" $RUN_LOG; then + echo "FAIL: dedicated-model build log line not found (reshard path not exercised)" + exit 1 +fi + +uv run tests/json_dump_tb_logs.py $LOG_DIR --output_path $JSON_METRICS + +# Observed to be between 0.8-1.3 +uv run tests/check_metrics.py $JSON_METRICS \ + 'median(data["train/gen_kl_error"]) < 1.3' \ + 'max(data["train/reward"]) > 0' diff --git a/tests/functional/grpo_megatron_generation_gym_single_controller.sh b/tests/functional/grpo_megatron_generation_gym_single_controller.sh new file mode 100755 index 00000000000..3566569d677 --- /dev/null +++ b/tests/functional/grpo_megatron_generation_gym_single_controller.sh @@ -0,0 +1,116 @@ +#!/bin/bash +# SingleController + NeMo-Gym + Megatron generation e2e smoke. +# Mirrors grpo_async_gym_single_controller.sh +# with the generation backend swapped to non-colocated Megatron Inference. + +SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd) +PROJECT_ROOT=$(realpath $SCRIPT_DIR/../..) +# Mark the current repo as safe, since wandb fetches metadata about the repo +git config --global --add safe.directory $PROJECT_ROOT + +set -eou pipefail + +EXP_NAME=$(basename $0 .sh) +EXP_DIR=$SCRIPT_DIR/$EXP_NAME +LOG_DIR=$EXP_DIR/logs +JSON_METRICS=$EXP_DIR/metrics.json +RUN_LOG=$EXP_DIR/run.log +CHECKPOINT_DIR=$EXP_DIR/checkpoints +DATA_DIR=$EXP_DIR/data +export PYTHONPATH=${PROJECT_ROOT}:${PYTHONPATH:-} + +rm -rf $EXP_DIR $LOG_DIR +mkdir -p $EXP_DIR $LOG_DIR $CHECKPOINT_DIR $DATA_DIR + +# clean up checkpoint directory on exit +trap "rm -rf $CHECKPOINT_DIR" EXIT + +cd $PROJECT_ROOT + +# Follow nemo-gym instructions here to get this data: +# https://docs.nvidia.com/nemo/gym/0.1.0/tutorials/nemo-rl-grpo/setup.html#training-nemo-rl-grpo-setup +cd 3rdparty/Gym-workspace/Gym + +# We need HF_TOKEN to download the data from huggingface +if [[ ! -f env.yaml ]]; then + if [[ -z "${HF_TOKEN:-}" ]]; then + echo "[ERROR] HF_TOKEN is not set" + exit 1 + fi + echo "hf_token: $HF_TOKEN" >> env.yaml +fi + +uv run ng_prepare_data "+config_paths=[resources_servers/workplace_assistant/configs/workplace_assistant.yaml]" \ + +output_dirpath=data/workplace_assistant \ + +mode=train_preparation \ + +should_download=true \ + +data_source=huggingface +cd - + +# This trimming of the workplace assistant dataset is necessary b/c with all the tools the first prompt is >4000 tokens +# which will cause the generation engine to return nothing on the first prompt and crash RL. Since we want to keep this test short to +# smoke test, we trim all but the first tool +TRAIN_PATH=$DATA_DIR/workplace_assistant_train.jsonl +VALIDATION_PATH=$DATA_DIR/workplace_assistant_validation.jsonl +jq -c '.responses_create_params.tools |= (.[0:1])' 3rdparty/Gym-workspace/Gym/data/workplace_assistant/train.jsonl > $TRAIN_PATH +jq -c '.responses_create_params.tools |= (.[0:1])' 3rdparty/Gym-workspace/Gym/data/workplace_assistant/validation.jsonl > $VALIDATION_PATH + +uv run coverage run -a --data-file=$PROJECT_ROOT/tests/.coverage --source=$PROJECT_ROOT/nemo_rl \ + $PROJECT_ROOT/examples/run_grpo_single_controller.py \ + --config $PROJECT_ROOT/examples/nemo_gym/grpo_qwen3_30ba3b_instruct.yaml \ + policy.model_name=Qwen/Qwen3-0.6B \ + policy.dtensor_cfg.enabled=false \ + policy.megatron_cfg.enabled=true \ + policy.megatron_cfg.tensor_model_parallel_size=1 \ + policy.megatron_cfg.pipeline_model_parallel_size=1 \ + policy.megatron_cfg.expert_model_parallel_size=1 \ + policy.megatron_cfg.context_parallel_size=1 \ + policy.megatron_cfg.sequence_parallel=false \ + policy.generation.backend=megatron \ + policy.generation.mcore_generation_config.expose_http_server=true \ + policy.generation.max_new_tokens=128 \ + policy.max_total_sequence_length=512 \ + policy.generation.colocated.enabled=false \ + policy.generation.colocated.resources.num_nodes=1 \ + policy.generation.colocated.resources.gpus_per_node=1 \ + grpo.num_prompts_per_step=4 \ + grpo.num_generations_per_prompt=2 \ + grpo.max_num_steps=10 \ + grpo.val_period=-1 \ + grpo.val_at_start=false \ + grpo.async_grpo=null \ + policy.train_global_batch_size=8 \ + policy.train_micro_batch_size=1 \ + cluster.gpus_per_node=2 \ + loss_fn.reference_policy_kl_penalty=0.01 \ + grpo.skip_reference_policy_logprobs_calculation=false \ + loss_fn.use_importance_sampling_correction=true \ + logger.tensorboard_enabled=true \ + logger.log_dir=$LOG_DIR \ + logger.wandb_enabled=false \ + logger.monitor_gpus=true \ + checkpointing.enabled=false \ + data.train.data_path=$TRAIN_PATH \ + data.validation.data_path=$VALIDATION_PATH \ + ++data_plane.enabled=true \ + ++data_plane.impl=transfer_queue \ + ++data_plane.backend=simple \ + ++data_plane.storage_capacity=1000000 \ + ++data_plane.num_storage_units=2 \ + ++data_plane.claim_meta_poll_interval_s=0.5 \ + ++data_plane.global_segment_size=549755813888 \ + ++data_plane.local_buffer_size=68719476736 \ + ++async_rl.sampler.name=in_order \ + ++async_rl.sampler.max_lookahead_versions=0 \ + ++async_rl.min_groups_for_streaming_train=4 \ + ++async_rl.max_inflight_prompts=4 \ + ++async_rl.max_buffered_rollouts=4 \ + $@ \ + 2>&1 | tee $RUN_LOG + +uv run tests/json_dump_tb_logs.py $LOG_DIR --output_path $JSON_METRICS + +# Observed to be between 0.8-1.3 +uv run tests/check_metrics.py $JSON_METRICS \ + 'median(data["train/gen_kl_error"]) < 1.3' \ + 'max(data["train/reward"]) > 0' diff --git a/tests/unit/algorithms/test_metric_utils.py b/tests/unit/algorithms/test_metric_utils.py index 42a55619b42..15359689b9d 100644 --- a/tests/unit/algorithms/test_metric_utils.py +++ b/tests/unit/algorithms/test_metric_utils.py @@ -25,7 +25,7 @@ class TestPrintSetupTimingSummary: - """print_setup_timing_summary has three code paths with assertions.""" + """print_setup_timing_summary's code paths and assertions.""" @staticmethod def _common_setup(**overrides) -> SetupTimingMetrics: @@ -37,8 +37,8 @@ def _common_setup(**overrides) -> SetupTimingMetrics: base.update(overrides) return SetupTimingMetrics(**base) - def test_sc_gym_on_prints_reserve_load_split(self, capsys): - """SC + gym-on renders the '(reserve X.Xs + load Y.Ys)' suffix.""" + def test_gym_on_prints_reserve_load_split(self, capsys): + """Gym-on (a reserved address) renders the '(reserve X.Xs + load Y.Ys)' suffix.""" metrics = self._common_setup( generation_init_time_s=15.0, generation_init_reserve_time_s=3.0, @@ -48,8 +48,8 @@ def test_sc_gym_on_prints_reserve_load_split(self, capsys): out = capsys.readouterr().out assert "Generation init: 15.0s (reserve 3.0s + load 12.0s)" in out - def test_sc_gym_off_prints_plain_generation_init(self, capsys): - """SC + gym-off renders only the top-level generation_init_time_s.""" + def test_gym_off_prints_plain_generation_init(self, capsys): + """Gym-off renders only the top-level generation_init_time_s.""" metrics = self._common_setup(generation_init_time_s=15.0) print_setup_timing_summary(metrics) out = capsys.readouterr().out @@ -58,40 +58,12 @@ def test_sc_gym_off_prints_plain_generation_init(self, capsys): assert "reserve" not in out assert "load" not in out - def test_sc_gym_off_asserts_generation_init_time_populated(self): - """SC path with gen_init_time_key=None must have generation_init_time_s set.""" + def test_asserts_generation_init_time_populated(self): + """Every driver must populate generation_init_time_s.""" metrics = self._common_setup() with pytest.raises(AssertionError): print_setup_timing_summary(metrics) - def test_grpo_uses_backend_specific_key(self, capsys): - """grpo.py path reads the field named by gen_init_time_key.""" - metrics = self._common_setup(vllm_init_time_s=15.0) - print_setup_timing_summary(metrics, gen_init_time_key="vllm_init_time_s") - out = capsys.readouterr().out - assert "Generation init: 15.0s" in out - assert "reserve" not in out - - def test_grpo_asserts_generation_init_time_unset(self): - """grpo.py path forbids generation_init_time_s from being populated.""" - metrics = self._common_setup( - generation_init_time_s=15.0, - vllm_init_time_s=15.0, - ) - with pytest.raises(AssertionError): - print_setup_timing_summary(metrics, gen_init_time_key="vllm_init_time_s") - - def test_reserve_load_split_takes_precedence_over_gen_key(self, capsys): - """If reserve_time_s is set, the SC+gym-on branch wins even if a key is passed.""" - metrics = self._common_setup( - generation_init_time_s=15.0, - generation_init_reserve_time_s=3.0, - generation_init_load_time_s=12.0, - ) - print_setup_timing_summary(metrics, gen_init_time_key="vllm_init_time_s") - out = capsys.readouterr().out - assert "Generation init: 15.0s (reserve 3.0s + load 12.0s)" in out - def test_optional_nemo_gym_and_teacher_lines(self, capsys): """nemo_gym_init_time_s and teacher_init_time_s only print when populated.""" metrics = self._common_setup( diff --git a/tests/unit/environments/test_nemo_gym_token_capture.py b/tests/unit/environments/test_nemo_gym_token_capture.py index aa564ec10f2..21cf3568ecf 100644 --- a/tests/unit/environments/test_nemo_gym_token_capture.py +++ b/tests/unit/environments/test_nemo_gym_token_capture.py @@ -17,7 +17,9 @@ import asyncio from unittest.mock import AsyncMock -from nemo_rl.environments.nemo_gym import NemoGym +import pytest + +from nemo_rl.environments.nemo_gym import NemoGym, _external_staging_backend def _capture_env() -> NemoGym: @@ -25,7 +27,33 @@ def _capture_env() -> NemoGym: return object.__new__(env_cls) -def _manifest_record(call_id: str, *, logical_request_id: str, parent: str | None = None) -> dict: +@pytest.mark.parametrize( + ("generation_backend", "expected"), + [("vllm", "vllm_worker"), ("megatron", "megatron_ledger")], +) +def test_external_staging_backend_maps_generation_backend( + generation_backend: str, expected: str +) -> None: + token_capture = {"generation_backend": generation_backend} + + assert _external_staging_backend(token_capture) == expected + assert token_capture == {"generation_backend": generation_backend} + + +@pytest.mark.parametrize( + "token_capture", + [{}, {"generation_backend": None}, {"generation_backend": "sglang"}], +) +def test_external_staging_backend_rejects_missing_or_invalid_backend( + token_capture: dict, +) -> None: + with pytest.raises(ValueError, match="setup-derived generation_backend"): + _external_staging_backend(token_capture) + + +def _manifest_record( + call_id: str, *, logical_request_id: str, parent: str | None = None +) -> dict: prev_len = 0 if parent is None else 900 return { "model_call_id": call_id, @@ -38,10 +66,30 @@ def _manifest_record(call_id: str, *, logical_request_id: str, parent: str | Non "extras_digest": "b" * 64, "staging_key": f"r0/{call_id}", "mode": "text" if parent is None else "token_in", + "chain_hash": "c" * 64, + "cumulative_hash": "d" * 64, + "response_id": f"resp-{call_id}", "logical_request_id": logical_request_id, } +def _pending_record( + call_id: str, *, logical_request_id: str, parent: str | None = None +) -> dict: + record = _manifest_record( + call_id, logical_request_id=logical_request_id, parent=parent + ) + for field in ( + "weight_version", + "digest", + "extras_digest", + "staging_key", + ): + record.pop(field) + record["ledger_request_uid"] = f"minf-{call_id}" + return record + + def test_receipt_postprocess_without_a_terminal_logical_id_uses_the_heuristic() -> None: env = _capture_env() records = [ @@ -98,6 +146,29 @@ def test_receipt_postprocess_fetches_manifest_and_selects_terminal_row() -> None assert [r["model_call_id"] for r in receipt["manifest"]] == ["c1", "c2"] +def test_receipt_assembly_preserves_pending_megatron_manifest() -> None: + env = _capture_env() + pending = [ + _pending_record("c1", logical_request_id="lr-1"), + _pending_record("c2", logical_request_id="lr-2", parent="c1"), + ] + receipt = env._assemble_receipt( + "r0", + { + "rollout_id": "r0", + "records": [], + "pending_records": pending, + "failures": [], + }, + terminal_logical_request_id="lr-2", + reward=1.0, + ) + assert receipt["manifest"] == [] + assert receipt["pending_manifest"] == pending + assert receipt["terminal_model_call_id"] == "c2" + assert receipt["capture_poisoned"] is False + + def test_receipt_assembly_poisons_on_failure_rows() -> None: env = _capture_env() manifest = { @@ -112,7 +183,9 @@ def test_receipt_assembly_poisons_on_failure_rows() -> None: assert receipt["failure_reason"] == "worker_capture_failed" -def test_receipt_assembly_ignores_uncommitted_call_failures_off_the_terminal_chain() -> None: +def test_receipt_assembly_ignores_uncommitted_call_failures_off_the_terminal_chain() -> ( + None +): """A call that died without coordinates never served a completion and can never be a lineage parent (no committed row to resolve against), so it is structurally off-chain — e.g. the doomed final call of a rollout that @@ -139,7 +212,9 @@ def test_receipt_assembly_ignores_uncommitted_call_failures_off_the_terminal_cha assert receipt["terminal_model_call_id"] == "c2" -def test_receipt_assembly_still_poisons_when_the_terminal_call_died_uncommitted() -> None: +def test_receipt_assembly_still_poisons_when_the_terminal_call_died_uncommitted() -> ( + None +): """If the reported terminal request itself died without coordinates there is no terminal row — the missing-terminal check must mask the rollout.""" env = _capture_env() diff --git a/tests/unit/models/generation/test_megatron_generation.py b/tests/unit/models/generation/test_megatron_generation.py index 873e4d419b3..cf2385a1eaa 100644 --- a/tests/unit/models/generation/test_megatron_generation.py +++ b/tests/unit/models/generation/test_megatron_generation.py @@ -14,6 +14,7 @@ import gc from copy import deepcopy +from types import SimpleNamespace import pytest import ray @@ -23,7 +24,10 @@ from nemo_rl.algorithms.utils import get_tokenizer from nemo_rl.distributed.batched_data_dict import BatchedDataDict from nemo_rl.distributed.virtual_cluster import RayVirtualCluster -from nemo_rl.models.generation.megatron import MegatronGeneration +from nemo_rl.models.generation.megatron import MegatronGeneration, megatron_generation +from nemo_rl.models.generation.megatron.config import ( + dedicated_inference_megatron_cfg, +) from nemo_rl.models.policy import PolicyConfig from nemo_rl.models.policy.lm_policy import Policy @@ -538,3 +542,112 @@ def test_megatron_generation_non_colocated_refit( print(f"Error during generation_cluster shutdown: {e}") gc.collect() torch.cuda.empty_cache() + + +class _CapturingPortHolder: + """Stand-in for the RemoteHeldPortReservation actor. + + Records the scheduling strategy it is pinned to and returns a fixed + (ip, port) instead of binding a real socket on a real placement group. + """ + + last_scheduling_strategy = None + + @classmethod + def options(cls, *, scheduling_strategy): + cls.last_scheduling_strategy = scheduling_strategy + return cls + + @classmethod + def remote(cls): + return SimpleNamespace( + address=SimpleNamespace(remote=lambda: ("10.0.0.5", 4321)) + ) + + +def _rank0_bundle_via_worker_group(sorted_bundle_indices, group_size): + """The bundle RANK 0 actually lands on, reconstructed from the live code. + + Mirrors lm_policy.py's tied_groups[0] for a unified PG and RayWorkerGroup's + default first-worker tuple otherwise -- the two branches + reserve_http_server_address must agree with. + + Deliberately a hand-copy rather than a call into the code under test (or a + shared helper): sharing the implementation would make the assertion a + tautology. The cost is that this copy is frozen -- if the worker-group + placement rule ever changes, update it by hand; grpo.py's runtime + served-vs-reserved URL check is the net for that direction. + """ + if sorted_bundle_indices is not None: + # lm_policy.py: tied_groups = [(i // group_size, [b]) for i, b in ...] + tied_groups = [ + (i // group_size, [bundle_idx]) + for i, bundle_idx in enumerate(sorted_bundle_indices) + ] + else: + # RayWorkerGroup.__init__: bundle_indices_list.append((i, [bundle_idx])) + # with i and bundle_idx both starting at 0. + tied_groups = [(0, [0])] + pg_idx, local_bundle_indices = tied_groups[0] + return pg_idx, local_bundle_indices[0] + + +@pytest.fixture +def patched_holder(monkeypatch): + monkeypatch.setattr( + megatron_generation, "RemoteHeldPortReservation", _CapturingPortHolder + ) + # ray.get here only unwraps the holder's (ip, port); no real Ray involved. + monkeypatch.setattr(megatron_generation.ray, "get", lambda ref: ref) + _CapturingPortHolder.last_scheduling_strategy = None + return _CapturingPortHolder + + +@pytest.mark.parametrize( + "sorted_bundle_indices", + [ + # Unified cross-node PG: topology sort can make rank 0 a bundle other + # than 0, so a naive "bundle 0" prediction would bind the wrong node. + [3, 1, 0, 2], + # Per-node PGs: no sorted indices, rank 0 is bundle 0 of the first PG. + None, + ], +) +def test_reserve_http_server_address_pins_rank0_bundle( + patched_holder, sorted_bundle_indices +): + """reserve_http_server_address's rank-0 prediction must match real placement. + + reserve_http_server_address publishes the OpenAI server URL to NeMo Gym + *before* any worker exists, pinning a port holder to the (placement_group, + bundle) it predicts rank 0 will occupy. That prediction is a second, + hand-written copy of the rank-0 placement lm_policy.py / RayWorkerGroup + actually perform: if the two ever disagree the holder binds the wrong node, + the pre-published URL is unreachable, and grpo.py fails loud at runtime. + Pins the prediction to a hand-reconstruction of that placement so the two + copies cannot silently drift -- no GPU or mcore extra needed + (MegatronGeneration imports without megatron.core). + """ + placement_groups = ["PG0", "PG1"] + cluster = SimpleNamespace( + num_gpus_per_node=8, + _sorted_bundle_indices=sorted_bundle_indices, + get_placement_groups=lambda: placement_groups, + ) + config = {"generation": {"colocated": {"enabled": True}}} + + url, port, holder = MegatronGeneration.reserve_http_server_address(cluster, config) + + expected_pg_idx, expected_bundle_index = _rank0_bundle_via_worker_group( + sorted_bundle_indices, cluster.num_gpus_per_node + ) + strategy = patched_holder.last_scheduling_strategy + # The holder -- and thus the pre-published URL's node -- must sit on the + # exact (placement_group, bundle) rank 0 will occupy. + assert expected_pg_idx == 0 # rank 0 is always in the first placement group + assert strategy.placement_group is placement_groups[expected_pg_idx] + assert strategy.placement_group_bundle_index == expected_bundle_index + + assert url == "http://10.0.0.5:4321/v1" + assert port == 4321 + assert holder.address.remote() == ("10.0.0.5", 4321) diff --git a/tests/unit/models/generation/test_megatron_generation_parse.py b/tests/unit/models/generation/test_megatron_generation_parse.py index 12fc5bcc105..6db700ef237 100644 --- a/tests/unit/models/generation/test_megatron_generation_parse.py +++ b/tests/unit/models/generation/test_megatron_generation_parse.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""CPU tests for packing mcore inference replies into a GenerationOutputSpec. +"""CPU tests for the GPU-free slices of the megatron generation worker. The end-to-end megatron generation tests need GPUs and several minutes, which makes them a poor guard for the wire format of the engine's reply. That format @@ -21,15 +21,30 @@ test began failing with `AttributeError: 'NoneType' object has no attribute 'tolist'` -- while two async ones swallowed the error per sample and still reported success. These tests pin the packing logic without a GPU. + +The same reasoning covers the NeMo-Gym port-reservation wiring +(test_http_server_port_reservation): its safety properties are pure +socket/plumbing contracts, pinned here without a GPU. """ +import socket from types import SimpleNamespace +from unittest.mock import MagicMock import pytest import torch +from megatron.core.inference.text_generation_server.dynamic_text_gen_server import ( + text_generation_server as mlm_text_gen_server, +) from nemo_rl.distributed.batched_data_dict import BatchedDataDict -from nemo_rl.models.generation.megatron.megatron_worker import MegatronGenerationMixin +from nemo_rl.distributed.held_port import ( + HeldPortReservation, + receive_held_socket, +) +from nemo_rl.models.generation.megatron.megatron_worker import ( + MegatronGenerationMixin, +) PAD = 0 @@ -150,3 +165,76 @@ def test_handles_a_reply_with_no_generated_tokens(two_sample_batch): torch.testing.assert_close(out["generation_lengths"], torch.tensor([0, 1])) torch.testing.assert_close(out["unpadded_sequence_lengths"], torch.tensor([3, 3])) assert out["output_ids"][0].tolist() == [5, 6, 7, PAD, PAD] + + +@pytest.mark.mcore +def test_http_server_port_reservation(monkeypatch): + """The NeMo-Gym overlap contract, exercised back to back. + + The server URL is published to NeMo Gym before any worker exists, so: the + reserved port is bound and listening from reservation time (early Gym + probes queue instead of being refused), the worker adopts that same socket + through the fd handoff — the port is never released in between — and the + server falls back to a fresh port only when nothing was reserved. + """ + # The holder resolves the node IP via held_port; the server resolves it via + # virtual_cluster (megatron_worker imports it at call time). Patch both. + monkeypatch.setattr( + "nemo_rl.distributed.held_port._get_node_ip_local", + lambda: "10.0.0.5", + ) + monkeypatch.setattr( + "nemo_rl.distributed.virtual_cluster._get_node_ip_local", + lambda: "10.0.0.5", + ) + holder = HeldPortReservation() + node_ip, port = holder.address() + assert node_ip == "10.0.0.5" + + # Held and listening from reservation time: early Gym probes queue + # instead of being refused. + with socket.create_connection(("127.0.0.1", port), timeout=5): + pass + + # Worker-side adoption: the same live socket, duplicated across the + # process boundary; still the same port, still accepting. + reserved = receive_held_socket(port) + try: + assert reserved.getsockname()[1] == port + with socket.create_connection(("127.0.0.1", port), timeout=5): + pass + + # Server start with the network and MLM server stubbed out. + started = {} + monkeypatch.setattr( + mlm_text_gen_server, + "start_text_gen_server", + lambda **kwargs: started.update(kwargs), + ) + monkeypatch.setattr( + "nemo_rl.distributed.virtual_cluster._get_free_port_local", + lambda: 12345, + ) + monkeypatch.setattr(torch.distributed, "get_rank", lambda: 0) + requests_mock = MagicMock() + health_get = requests_mock.Session.return_value.__enter__.return_value.get + health_get.return_value.status_code = 200 + monkeypatch.setattr( + "nemo_rl.models.generation.megatron.megatron_worker.requests", + requests_mock, + ) + + for reserved_socket, expected_port in ((reserved, port), (None, 12345)): + worker = SimpleNamespace( + coordinator_addr="tcp://127.0.0.1:5555", + megatron_tokenizer=object(), + rank=0, + cfg={"generation": {"mcore_generation_config": {"parsers": []}}}, + _reserved_http_server_socket=reserved_socket, + ) + base_url = MegatronGenerationMixin._setup_openai_api_server(worker) + assert started["sock"] is reserved_socket + assert started["server_port"] == expected_port + assert base_url == f"http://10.0.0.5:{expected_port}/v1" + finally: + reserved.close() diff --git a/tests/unit/models/generation/test_megatron_token_capture_hosting.py b/tests/unit/models/generation/test_megatron_token_capture_hosting.py new file mode 100644 index 00000000000..f14d8b8ce47 --- /dev/null +++ b/tests/unit/models/generation/test_megatron_token_capture_hosting.py @@ -0,0 +1,316 @@ +# Copyright (c) 2026, NVIDIA CORPORATION. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import annotations + +import threading +from types import SimpleNamespace +from typing import Any + +import pytest + +nemo_gym = pytest.importorskip("nemo_gym.token_id_capture.staging") + +from nemo_gym.token_id_capture.adapters.megatron import ( # noqa: E402 + MegatronCaptureAdapter, +) +from nemo_gym.token_id_capture.staging.capture import ( # noqa: E402 + RolloutTokenCapture, +) +from nemo_gym.token_id_capture.staging.digest import ( # noqa: E402 + compute_chain_hash, + hash_token_ids, +) +from nemo_gym.token_id_capture.staging.records import ( # noqa: E402 + StagedCallRecord, + StageResult, +) + +from nemo_rl.models.generation.megatron.megatron_generation import ( # noqa: E402 + MegatronGeneration, +) +from nemo_rl.models.generation.megatron.megatron_worker import ( # noqa: E402 + MegatronGenerationMixin, +) + +pytestmark = pytest.mark.nemo_gym + + +class _MemorySink: + def __init__(self, *, reject: bool = False) -> None: + self.reject = reject + self.records: list[StagedCallRecord] = [] + + def stage(self, record: StagedCallRecord) -> StageResult: + if self.reject: + return StageResult(ok=False, error="rejected") + self.records.append(record) + return StageResult(ok=True, staging_key=record.staging_key) + + +class _DeferredAdapter: + """Structural deferred adapter that is deliberately not the MInf concrete type.""" + + def __init__(self) -> None: + self._adapter = MegatronCaptureAdapter() + + def enter_prefix( + self, request_payload: dict[str, Any], prefix_ids: list[int] + ) -> dict[str, Any]: + return self._adapter.enter_prefix(request_payload, prefix_ids) + + def extract_prompt_ids(self, response_payload: dict[str, Any]) -> list[int]: + return self._adapter.extract_prompt_ids(response_payload) + + def extract_generation( + self, response_payload: dict[str, Any] + ) -> tuple[list[int], list[float]]: + return self._adapter.extract_generation(response_payload) + + def extract_extras(self, response_payload: dict[str, Any]) -> dict[str, Any] | None: + return self._adapter.extract_extras(response_payload) + + def extract_weight_version(self, response_payload: dict[str, Any]) -> int: + return self._adapter.extract_weight_version(response_payload) + + +class _WorkerGroup: + def __init__(self, ledger: dict[str, dict]) -> None: + self.ledger = ledger + self.calls: list[tuple[str, dict]] = [] + + def run_all_workers_single_data(self, method_name: str, **kwargs): + self.calls.append((method_name, kwargs)) + if method_name == "fetch_token_capture_records": + return [ + { + uid: self.ledger[uid] + for uid in kwargs["request_uids"] + if uid in self.ledger + }, + {}, + ] + if method_name == "discard_token_capture_records": + for uid in kwargs["request_uids"]: + self.ledger.pop(uid, None) + return [len(kwargs["request_uids"]), 0] + return [True, True] + + +def _generation( + monkeypatch: pytest.MonkeyPatch, + sink: _MemorySink, + ledger: dict[str, dict], + adapter: Any | None = None, +) -> MegatronGeneration: + generation = object.__new__(MegatronGeneration) + generation._policy = SimpleNamespace(worker_group=_WorkerGroup(ledger)) + generation._token_capture = RolloutTokenCapture( + sink=sink, + weight_version_fn=lambda: 0, + adapter=adapter or MegatronCaptureAdapter(), + ) + generation._token_capture_flush_lock = threading.Lock() + monkeypatch.setattr( + "nemo_rl.models.generation.megatron.megatron_generation.ray.get", + lambda value: value, + ) + return generation + + +def _receipt() -> dict: + token_ids = [10, 11, 12] + return { + "rollout_id": "r0", + "reward": 1.0, + "terminal_model_call_id": "c1", + "manifest": [], + "pending_manifest": [ + { + "model_call_id": "c1", + "parent_call_id": None, + "prev_len": 0, + "delta_len": 3, + "cum_len": 3, + "mode": "text", + "ledger_request_uid": "minf-1", + "chain_hash": compute_chain_hash(None, token_ids), + "cumulative_hash": hash_token_ids(token_ids), + "response_id": "minf-1", + "logical_request_id": "lr-1", + } + ], + "capture_poisoned": False, + "failure_reason": None, + "terminal_selection": "declared", + } + + +def _ledger_record() -> dict: + return { + "policy_epoch": [[0, 7]], + "kv_cache_epoch": None, + "num_evictions": 0, + "prompt_token_ids": [10, 11], + "generated_token_ids": [12], + "generated_log_probs": [-0.25], + "prompt_log_probs": None, + "routing_indices": None, + } + + +def test_rollout_flush_stages_whole_ledger_batch_then_discards(monkeypatch): + sink = _MemorySink() + ledger = {"minf-1": _ledger_record()} + generation = _generation(monkeypatch, sink, ledger) + + finalized = generation.flush_token_capture(_receipt()) + + assert "pending_manifest" not in finalized + assert finalized["manifest"][0]["weight_version"] == 7 + assert finalized["manifest"][0]["staging_key"] == "r0/c1" + assert finalized["manifest"][0]["response_id"] == "minf-1" + assert sink.records[0].token_ids_delta == [10, 11, 12] + assert ledger == {} + assert [call[0] for call in generation._policy.worker_group.calls] == [ + "fetch_token_capture_records", + "discard_token_capture_records", + ] + + +def test_rollout_flush_accepts_a_structural_deferred_adapter( + monkeypatch: pytest.MonkeyPatch, +) -> None: + sink = _MemorySink() + ledger = {"minf-1": _ledger_record()} + generation = _generation(monkeypatch, sink, ledger, adapter=_DeferredAdapter()) + + finalized = generation.flush_token_capture(_receipt()) + + assert finalized["manifest"][0]["weight_version"] == 7 + assert sink.records[0].token_ids_delta == [10, 11, 12] + + +def test_rollout_flush_preserves_multiturn_chain_custody( + monkeypatch: pytest.MonkeyPatch, +) -> None: + root_tokens = [10, 11, 12] + child_delta = [13, 14] + root_chain_hash = compute_chain_hash(None, root_tokens) + child_chain_hash = compute_chain_hash(root_chain_hash, child_delta) + receipt = _receipt() + receipt["terminal_model_call_id"] = "c2" + receipt["pending_manifest"].append( + { + "model_call_id": "c2", + "parent_call_id": "c1", + "prev_len": len(root_tokens), + "delta_len": len(child_delta), + "cum_len": len(root_tokens) + len(child_delta), + "mode": "token_in", + "ledger_request_uid": "minf-2", + "chain_hash": child_chain_hash, + "cumulative_hash": hash_token_ids(root_tokens + child_delta), + "response_id": "minf-2", + "logical_request_id": "lr-2", + } + ) + child_ledger_record = _ledger_record() + child_ledger_record["prompt_token_ids"] = root_tokens + [13] + child_ledger_record["generated_token_ids"] = [14] + child_ledger_record["generated_log_probs"] = [-0.5] + sink = _MemorySink() + generation = _generation( + monkeypatch, + sink, + {"minf-1": _ledger_record(), "minf-2": child_ledger_record}, + ) + + finalized = generation.flush_token_capture(receipt) + + assert [record["response_id"] for record in finalized["manifest"]] == [ + "minf-1", + "minf-2", + ] + assert finalized["manifest"][1]["chain_hash"] == child_chain_hash + assert sink.records[1].token_ids_delta == child_delta + + +def test_rollout_flush_keeps_ledger_when_staging_fails(monkeypatch): + sink = _MemorySink(reject=True) + ledger = {"minf-1": _ledger_record()} + generation = _generation(monkeypatch, sink, ledger) + + with pytest.raises(RuntimeError, match="failed to stage"): + generation.flush_token_capture(_receipt()) + + assert "minf-1" in ledger + assert [call[0] for call in generation._policy.worker_group.calls] == [ + "fetch_token_capture_records" + ] + + +def test_rollout_flush_validates_http_lengths_before_staging(monkeypatch): + sink = _MemorySink() + ledger = {"minf-1": _ledger_record()} + generation = _generation(monkeypatch, sink, ledger) + receipt = _receipt() + receipt["pending_manifest"][0]["delta_len"] = 4 + receipt["pending_manifest"][0]["cum_len"] = 4 + + with pytest.raises(RuntimeError, match="HTTP lineage lengths"): + generation.flush_token_capture(receipt) + + assert sink.records == [] + assert "minf-1" in ledger + assert [call[0] for call in generation._policy.worker_group.calls] == [ + "fetch_token_capture_records" + ] + + +def test_rollout_flush_validates_http_token_content_before_staging(monkeypatch): + sink = _MemorySink() + ledger_record = _ledger_record() + ledger_record["generated_token_ids"] = [99] + ledger = {"minf-1": ledger_record} + generation = _generation(monkeypatch, sink, ledger) + + with pytest.raises(RuntimeError, match="token content differs"): + generation.flush_token_capture(_receipt()) + + assert sink.records == [] + assert "minf-1" in ledger + assert [call[0] for call in generation._policy.worker_group.calls] == [ + "fetch_token_capture_records" + ] + + +def test_worker_enables_and_reads_minf_ledger() -> None: + record = SimpleNamespace(**_ledger_record()) + + class _Engine: + local_metadata_ledger_enabled = False + local_metadata_ledger_offload_enabled = False + + def fetch_from_metadata_ledger(self, uids, pop=True): + return {"minf-1": record} if "minf-1" in uids else {} + + worker = object.__new__(MegatronGenerationMixin) + worker.dynamic_inference_engine = _Engine() + worker._token_capture_enabled = False + assert worker.setup_token_capture() + assert worker.dynamic_inference_engine.local_metadata_ledger_enabled + assert worker.dynamic_inference_engine.local_metadata_ledger_offload_enabled + fetched = worker.fetch_token_capture_records(["minf-1"]) + assert fetched["minf-1"]["generated_token_ids"] == [12] diff --git a/tests/unit/single_controller/test_sc_checkpointing.py b/tests/unit/single_controller/test_sc_checkpointing.py index f1c7862ee7b..5d59e6a2e4b 100644 --- a/tests/unit/single_controller/test_sc_checkpointing.py +++ b/tests/unit/single_controller/test_sc_checkpointing.py @@ -365,6 +365,27 @@ def save_checkpoint( super().save_checkpoint(checkpoint_dir, metadata=metadata) +class _BlockingGeneration: + """Colocated stand-in: blocks training, and its wake carries the update.""" + + def __init__(self, events: list[str]) -> None: + self._events = events + + def blocks_training(self) -> bool: + return True + + def wake_carries_weight_updates(self) -> bool: + return True + + def finish_generation(self, *args: Any, **kwargs: Any) -> bool: + self._events.append("finish_generation") + return True + + def prepare_for_generation(self, *args: Any, **kwargs: Any) -> bool: + self._events.append("wake") + return True + + class _FakeWeightSynchronizer: def __init__(self) -> None: self.sync_count = 0 @@ -387,6 +408,47 @@ def set_rollout_weight_version(self, version: int) -> None: def drain_latest_logger_metrics(self) -> dict[str, Any]: return {} + def blocks_training(self) -> bool: + return False + + def wake_carries_weight_updates(self) -> bool: + return False + + +class _EventRecordingSynchronizer(_FakeWeightSynchronizer): + def __init__(self, events: list[str]) -> None: + super().__init__() + self._events = events + + def sync_weights(self, *, kv_scales: Any = None) -> None: + self._events.append("sync") + super().sync_weights(kv_scales=kv_scales) + + +class _RefitRecordingTrainer(_FakeTrainer): + """Records the offload calls the deferred-wake sync path makes.""" + + def __init__(self, events: list[str]) -> None: + super().__init__() + self._events = events + self.gate_open_during_save: list[bool] = [] + self._gate_probe: Optional[Callable[[], bool]] = None + + def set_gate_probe(self, probe: Callable[[], bool]) -> None: + self._gate_probe = probe + + def offload_before_refit(self) -> None: + self._events.append("offload_before_refit") + + def offload_after_refit(self) -> None: + self._events.append("offload_after_refit") + + def save_checkpoint(self, **kwargs: Any) -> None: + self._events.append("save") + if self._gate_probe is not None: + self.gate_open_during_save.append(self._gate_probe()) + super().save_checkpoint(**kwargs) + class _FakeRolloutManager: def __init__(self, recovery_ledger: Optional[RolloutRecoveryLedger] = None) -> None: @@ -637,6 +699,8 @@ def _actor_master_config( def _make_actor_args( *, trainer: Optional[_FakeTrainer] = None, + gen: Optional[Any] = None, + weight_synchronizer: Optional[_FakeWeightSynchronizer] = None, save_state: Optional[GRPOSaveState] = None, dataloader: Optional[_FakeDataloader] = None, tq_buffer: Optional[_FakeTQBuffer] = None, @@ -648,14 +712,18 @@ def _make_actor_args( rollout_checkpoint_load_metrics: Optional[dict[str, float]] = None, ) -> SingleControllerActorArgs: return SingleControllerActorArgs( - gen_handle=_FakeGeneration(), + gen_handle=gen if gen is not None else _FakeGeneration(), trainer_handle=trainer if trainer is not None else _FakeTrainer(), env_handles={}, train_cluster=None, # type: ignore[arg-type] inference_cluster=None, # type: ignore[arg-type] dp_client=dp_client if dp_client is not None else _FakeDPClient(), dataloader=dataloader if dataloader is not None else _FakeDataloader(), - weight_synchronizer=_FakeWeightSynchronizer(), # type: ignore[arg-type] + weight_synchronizer=( # type: ignore[arg-type] + weight_synchronizer + if weight_synchronizer is not None + else _FakeWeightSynchronizer() + ), advantage_estimator=None, loss_fn=object(), # type: ignore[arg-type] rollout_manager=( @@ -908,6 +976,54 @@ def test_saves_on_period_boundary_and_last_step(self, tmp_path): assert (ckpt_dir / "step_2" / "policy" / "optimizer").is_dir() assert (ckpt_dir / "step_4" / "policy" / "tokenizer").is_dir() + def test_blocking_engine_stays_down_through_saves(self, tmp_path): + """Save-bound steps defer the blocking engine's wake past the save. + + Four steps, save_period=2. Non-save steps wake through the + synchronizer as usual. The step-2 save runs against a stood-down + engine and a closed gate: offload_before_refit replaces the sync, + and offload_after_refit + wake follow the save. The step-4 save is + on the last step, so the wake is skipped and the gate stays closed + for teardown. + """ + mc = _actor_master_config(tmp_path, max_num_steps=4, save_period=2) + events: list[str] = [] + trainer = _RefitRecordingTrainer(events) + gen = _BlockingGeneration(events) + synchronizer = _EventRecordingSynchronizer(events) + + actor = _run_train_pump( + mc, + _make_actor_args( + trainer=trainer, gen=gen, weight_synchronizer=synchronizer + ), + seed=lambda actor: trainer.set_gate_probe(actor._rollout_permitted.is_set), + ) + + assert _step_dir_names(tmp_path / "checkpoints") == {"step_2", "step_4"} + assert events == [ + # step 1: stand down, then the plain wake inside _sync_weights. + "finish_generation", + "sync", + # step 2: save-bound — no sync; the wake is deferred past the save. + "finish_generation", + "offload_before_refit", + "save", + "offload_after_refit", + "wake", + # step 3: back to the plain shape. + "finish_generation", + "sync", + # step 4: save on the last step — no wake at all. + "finish_generation", + "offload_before_refit", + "save", + ] + # The gate was closed during both saves and never reopened after the + # final one (run()'s teardown cancels the pumps, so nothing hangs). + assert trainer.gate_open_during_save == [False, False] + assert not actor._rollout_permitted.is_set() + def test_last_step_saves_off_period_boundary(self, tmp_path): mc = _actor_master_config(tmp_path, max_num_steps=3, save_period=2) diff --git a/tests/unit/single_controller/test_single_controller.py b/tests/unit/single_controller/test_single_controller.py index 2d406dd73d1..372d7977e74 100644 --- a/tests/unit/single_controller/test_single_controller.py +++ b/tests/unit/single_controller/test_single_controller.py @@ -380,7 +380,11 @@ def _train_pump_controller(*, sampler) -> object: ctrl._rollout_recovery_complete = asyncio.Event() ctrl._rollout_recovery_complete.set() ctrl._trainer = _NoOpTrainer() - ctrl._gen = SimpleNamespace(requires_kv_scale_sync=False) + # Continuous-serving default; the pump asks before every step's trainer work. + ctrl._gen = SimpleNamespace( + requires_kv_scale_sync=False, + blocks_training=lambda: False, + ) ctrl._loss_fn = None ctrl._dp_client = _NoOpDataPlane() ctrl._timer = Timer() @@ -462,7 +466,103 @@ def test_train_pump_logs_nonzero_stale_group_metrics(monkeypatch) -> None: asyncio.run(asyncio.wait_for(ctrl._train_pump(), timeout=1.0)) - ctrl._sync_weights.assert_awaited_once_with(calibration_data=None) + ctrl._sync_weights.assert_awaited_once_with( + calibration_data=None, defer_engine_wake=False + ) train_metrics = ctrl._logger.log_metrics.call_args_list[0].args[0] assert train_metrics["evicted_stale_prompt_groups"] == 2 assert train_metrics["aborted_stale_inflight_groups"] == 1 + + +@pytest.mark.parametrize( + "engine_blocks_training", [False, True], ids=["streaming", "blocking"] +) +def test_train_pump_chunked_step_by_engine_regime( + monkeypatch, engine_blocks_training +) -> None: + """One two-chunk step, observed under both engine regimes. + + Both regimes: the logprob detour between chunks must not offload the + trainer's grad buffers — mcore's offload frees the gradients earlier + chunks accumulated rather than copying them out. First chunk: no step + open, the offload is worth taking; later chunks: buffers stay resident. + + Streaming: the engine is never stood down, the rollout gate stays open, + and the configured streaming minimum reaches the sampler. + + Blocking (colocated Megatron): the pump closes the gate and sleeps the + engine exactly once per step, before any trainer GPU work, and demands + whole steps from the sampler (min == max). The chunked delivery here is + a fake-permitted shape — real samplers honor the min — proving + release-once is robust to partial deliveries. + """ + meta = KVBatchMeta( + partition_id="rollout_data", + task_name="train", + sample_ids=["sample-0"], + fields=[], + sequence_lengths=[1], + tags=[{"weight_version": 0}], + ) + select_bounds: list[tuple[int, int]] = [] + + class _BoundsRecordingSampler(_ChunkedSampler): + async def select(self, **kwargs): + select_bounds.append( + (kwargs["min_prompt_groups"], kwargs["max_prompt_groups"]) + ) + return await super().select(**kwargs) + + # num_prompts_per_step is 2 in the harness, so two single-group chunks + # close the step. + ctrl = _train_pump_controller(sampler=_BoundsRecordingSampler(meta, chunks=2)) + ctrl._policy_logprobs_required = True + calls: list[object] = [] + + class _RecordingTrainer(_LpRecordingTrainer): + def prepare_for_lp_inference(self, keep_train_buffers: bool = False) -> None: + super().prepare_for_lp_inference(keep_train_buffers) + calls.append("lp_inference_prep") + + def prepare_for_training(self) -> None: + calls.append("prepare_for_training") + + def train_microbatches_from_meta(self, meta: KVBatchMeta) -> None: + del meta + calls.append("train") + + def _finish_generation() -> None: + calls.append(("finish_generation", ctrl._rollout_permitted.is_set())) + + trainer = _RecordingTrainer() + ctrl._trainer = trainer + ctrl._gen = SimpleNamespace( + requires_kv_scale_sync=False, + blocks_training=lambda: engine_blocks_training, + finish_generation=_finish_generation, + ) + ctrl._rollout_permitted = asyncio.Event() + ctrl._rollout_permitted.set() + ctrl._sync_weights = AsyncMock(return_value=0) + ctrl._logger = MagicMock() + monkeypatch.setattr(single_controller.ray, "cluster_resources", lambda: {}) + + asyncio.run(asyncio.wait_for(ctrl._train_pump(), timeout=1.0)) + + assert ctrl._train_steps == 1 + assert trainer.keep_train_buffers_calls == [False, True] + chunk = ["lp_inference_prep", "prepare_for_training", "train"] + if engine_blocks_training: + # Released exactly once, with the gate already closed, before the + # trainer touched the GPUs; both chunks then ran without a second + # release. The (mocked) post-step _sync_weights reopens the gate. + assert calls == [("finish_generation", False)] + chunk * 2 + assert not ctrl._rollout_permitted.is_set() + assert select_bounds and all(lo == hi for lo, hi in select_bounds) + else: + assert calls == chunk * 2 + assert ctrl._rollout_permitted.is_set() + assert select_bounds[0] == (1, 2) + ctrl._sync_weights.assert_awaited_once_with( + calibration_data=None, defer_engine_wake=False + ) diff --git a/tests/unit/single_controller/test_single_controller_setup.py b/tests/unit/single_controller/test_single_controller_setup.py index 8b739cdb182..6b2d6556d2f 100644 --- a/tests/unit/single_controller/test_single_controller_setup.py +++ b/tests/unit/single_controller/test_single_controller_setup.py @@ -55,6 +55,7 @@ PromptRef, RolloutRecoveryLedger, ) +from nemo_rl.models.generation.megatron.megatron_generation import MegatronGeneration class _CheckpointingCustomSampler(WindowedSampler): @@ -82,6 +83,21 @@ def _make_master_config( normal load but unused here — model_construct skips validation, and we hand-fill only the dict-shaped fields setup reads. """ + generation_config: dict = { + "backend": backend, + "colocated": {"enabled": colocated, "resources": {}}, + } + policy_config: dict = { + "train_global_batch_size": num_prompts_per_step * 2, + "max_total_sequence_length": 32, + "tokenizer": {"use_fastokens": False}, + "megatron_cfg": {"enabled": megatron_enabled}, + "generation": generation_config, + } + if backend == "megatron": + # The megatron build path reads these before any generation factory runs. + generation_config["mcore_generation_config"] = {"expose_http_server": False} + policy_config["model_name"] = "test-model" return MasterConfig.model_construct( data_plane={"enabled": dp_enabled, "impl": "transfer_queue"}, data={ @@ -101,16 +117,7 @@ def _make_master_config( val_at_start=False, val_at_end=False, ), - policy={ - "train_global_batch_size": num_prompts_per_step * 2, - "max_total_sequence_length": 32, - "tokenizer": {"use_fastokens": False}, - "megatron_cfg": {"enabled": megatron_enabled}, - "generation": { - "backend": backend, - "colocated": {"enabled": colocated, "resources": {}}, - }, - }, + policy=policy_config, # Full block: setup builds a CheckpointManager unconditionally (resume # lookup), which indexes these keys directly. Nothing is written while # enabled=False and the dir doesn't exist. @@ -161,6 +168,39 @@ def _save_state( return state +@pytest.mark.parametrize( + ("backend", "expected", "unexpected"), + [ + ( + "megatron", + "driver environment", + "VllmAsyncGenerationWorker environment", + ), + ( + "vllm", + "VllmAsyncGenerationWorker environment", + "driver environment", + ), + ], +) +def test_missing_nemo_gym_remediation_is_backend_specific( + backend: str, expected: str, unexpected: str +) -> None: + import_error = ModuleNotFoundError("No module named 'nemo_gym'") + + with pytest.raises(RuntimeError, match=expected) as exc_info: + sc_setup_mod._raise_missing_nemo_gym_error(import_error, backend) + + assert unexpected not in str(exc_info.value) + assert exc_info.value.__cause__ is import_error + if backend == "megatron": + assert "uv run --extra nemo_gym" in str(exc_info.value) + assert "uv sync --extra nemo_gym" in str(exc_info.value) + else: + assert "NRL_FORCE_REBUILD_VENVS=true" in str(exc_info.value) + assert "$NEMO_RL_VENV_DIR" in str(exc_info.value) + + @pytest.fixture def patched_factories(): """Patch every external factory setup calls. @@ -266,18 +306,6 @@ def test_build_generation_passes_sglang_config(): generation.finish_generation.assert_called_once_with() -def test_build_clusters_rejects_non_colocated_megatron_generation(): - """The topology guard identifies Megatron as the generation backend.""" - master_config = _make_master_config(colocated=False, backend="megatron") - master_config.cluster = {"num_nodes": 2, "gpus_per_node": 8} - - with pytest.raises( - AssertionError, - match="Megatron generation backend.*non-colocated inference", - ): - sc_setup_mod._build_clusters(master_config) - - class TestSetup: """setup arg validation + actor_args assembly.""" @@ -377,47 +405,65 @@ def test_multiple_dataloader_not_supported(self): setup_single_controller(mc, MagicMock(pad_token_id=0)) @pytest.mark.parametrize( - ("invalid_case", "match"), + ("invalid_case", "expected_error", "match"), [ - ("min_groups", "must be >="), - ("global_batch_size", "must equal policy.train_global_batch_size"), - ("buffer_capacity", "required capacity"), + ("min_groups", ValueError, "must be >="), + ( + "global_batch_size", + ValueError, + "must equal policy.train_global_batch_size", + ), + ("buffer_capacity", ValueError, "required capacity"), ( "streaming_buffer_capacity", + ValueError, "max_buffered_rollouts.*must be >=.*min_groups_for_streaming_train", ), ( "deferred_routes_without_capture", + ValueError, "defer_routed_experts_to_policy requires", ), ( "recovery_capacity", + ValueError, "num_prompts_per_step.*min_groups_for_streaming_train - 1", ), + ("megatron_dtensor_trainer", ValueError, "megatron_cfg.enabled"), + ("megatron_colocated_small_buffer", ValueError, "max_buffered_rollouts"), + ("megatron_gym_without_http_server", ValueError, "expose_http_server"), + ("gym_on_sglang", NotImplementedError, "vllm and megatron"), ], ) def test_invalid_config_fails_before_setup_factories( self, invalid_case: str, + expected_error: type[Exception], match: str, patched_factories, ): - mc = _make_master_config() + use_gym = invalid_case in ("megatron_gym_without_http_server", "gym_on_sglang") if invalid_case == "min_groups": + mc = _make_master_config() mc.async_rl.min_groups_for_streaming_train = 5 elif invalid_case == "global_batch_size": + mc = _make_master_config() mc.policy["train_global_batch_size"] = 7 elif invalid_case == "buffer_capacity": + mc = _make_master_config() mc.async_rl.max_buffered_rollouts = 7 elif invalid_case == "streaming_buffer_capacity": + mc = _make_master_config() # WindowedSampler has no stronger sampler-specific capacity floor. A # buffer smaller than the streaming threshold would let the producer # consume every permit while the trainer waits for an unreachable count. mc.async_rl.sampler = WindowedSamplerConfig(max_staleness_versions=1) mc.async_rl.max_buffered_rollouts = 3 elif invalid_case == "deferred_routes_without_capture": + mc = _make_master_config() mc.token_capture.defer_routed_experts_to_policy = True elif invalid_case == "recovery_capacity": + mc = _make_master_config() # Four restored groups may leave one group below the streaming # threshold while the producer atomically reserves the next full # four-prompt batch. Capacity 4 cannot make progress; 4 + 2 - 1 can. @@ -427,16 +473,37 @@ def test_invalid_config_fails_before_setup_factories( mc.token_capture.enabled = True mc.checkpointing["enabled"] = True mc.data_plane["checkpointing_enabled"] = True + elif invalid_case == "megatron_dtensor_trainer": + mc = _make_master_config( + colocated=False, backend="megatron", megatron_enabled=False + ) + elif invalid_case == "megatron_colocated_small_buffer": + mc = _make_master_config( + colocated=True, backend="megatron", megatron_enabled=True + ) + mc.async_rl.max_buffered_rollouts = mc.grpo.num_prompts_per_step - 1 + elif invalid_case == "megatron_gym_without_http_server": + mc = self._make_gym_megatron_config() + mc.policy["generation"]["mcore_generation_config"]["expose_http_server"] = ( + False + ) + elif invalid_case == "gym_on_sglang": + mc = _make_master_config(colocated=True, backend="sglang") else: # pragma: no cover raise AssertionError(f"unknown test case {invalid_case}") - with pytest.raises(ValueError, match=match): + with ( + patch.object(sc_setup_mod, "_should_use_nemo_gym", return_value=use_gym), + patch.object(sc_setup_mod, "spinup_nemo_gym_actor") as mock_spinup, + pytest.raises(expected_error, match=match), + ): setup_single_controller(mc, MagicMock(pad_token_id=0)) patched_factories["setup_response_data"].assert_not_called() patched_factories["_build_clusters"].assert_not_called() patched_factories["_build_generation"].assert_not_called() patched_factories["_build_trainer"].assert_not_called() + mock_spinup.assert_not_called() def test_returns_actor_args(self, patched_factories): mc = _make_master_config(colocated=True) @@ -644,7 +711,7 @@ def test_token_capture_always_creates_finalizer_actor_pool(self, patched_factori patch.object(sc_setup_mod, "_should_use_nemo_gym", return_value=True), patch.object( sc_setup_mod, "spinup_nemo_gym_actor", return_value=MagicMock() - ), + ) as mock_spinup, patch.object(sc_setup_mod, "router_replay_enabled", return_value=False), patch( "nemo_rl.experience.finalizer_actor.create_finalizer_actors", @@ -663,6 +730,10 @@ def test_token_capture_always_creates_finalizer_actor_pool(self, patched_factori assert actor_kwargs == {"num_workers": 3} assert actor_args.finalizer_actors == fake_actors assert not hasattr(actor_args.rollout_manager, "_finalizer") + assert mc.token_capture.generation_backend == "vllm" + assert mock_spinup.call_args.kwargs["token_capture"]["generation_backend"] == ( + "vllm" + ) def test_setup_timing_populated_for_colocated_vllm(self, patched_factories): """Colocated vLLM records gen+policy+collective+total+worker fields.""" @@ -712,9 +783,6 @@ def test_setup_timing_backend_agnostic_for_sglang(self, patched_factories): _, metrics = setup_single_controller(mc, MagicMock(pad_token_id=0)) assert metrics.generation_init_time_s is not None - # Backend-specific fields are grpo.py-only; SC does not populate them. - assert metrics.vllm_init_time_s is None - assert metrics.sglang_init_time_s is None def test_nemo_gym_uses_deferred_vllm_load(self, patched_factories): """NeMo-Gym path reserves vLLM ports up-front and finishes the load afterwards.""" @@ -837,22 +905,160 @@ def test_nemo_gym_generation_init_time_includes_reserve_time( assert metrics.generation_init_reserve_time_s == 3.0 assert metrics.generation_init_load_time_s is not None - @pytest.mark.parametrize("backend", ["sglang", "megatron"]) - def test_nemo_gym_rejects_non_vllm_backend(self, patched_factories, backend): - """SC nemo-gym wiring only supports vLLM; every other backend must raise.""" - mc = _make_master_config(colocated=True, backend=backend) - patched_factories["setup_response_data"].return_value = ( - list(range(8)), - None, + def _make_gym_megatron_config(self, *, colocated: bool = False) -> MasterConfig: + mc = _make_master_config( + colocated=colocated, backend="megatron", megatron_enabled=True + ) + mc.policy["generation"]["mcore_generation_config"]["expose_http_server"] = True + mc.policy["generation"]["stop_strings"] = None + mc.policy["generation"]["stop_token_ids"] = None + mc.policy["generation"]["top_k"] = None + return mc + + @pytest.mark.parametrize("colocated", [True, False]) + @pytest.mark.parametrize( + ("scenario", "error_match"), + [ + ("gym", None), + ("gym_served_mismatch", "different address"), + ("native", None), + ], + ids=["gym", "gym_served_mismatch", "native"], + ) + def test_megatron_setup( + self, + patched_factories, + scenario: str, + error_match: str | None, + colocated: bool, + ): + """Megatron generation setup: gym and native legs, colocated or not. + + gym: reserve rank-0's URL, spin Gym up on it, build trainer + engine + (weight load skipped, reserved port adopted), cross-check the served + address, reap the port holder. + gym_served_mismatch: the served-vs-reserved cross-check fires after the + builds when the engine comes up on a different address. + native: expose_http_server=false and no Gym, so nothing reserves a URL, + no port holder is created, and the cross-check is skipped. + colocated: rank 0 lives with the trainer — the reservation targets the + train cluster, the reserved port rides the trainer build, and the + engine wraps the trainer's policy instead of a dedicated cluster. + """ + gym = scenario != "native" + if gym: + mc = self._make_gym_megatron_config(colocated=colocated) + patched_factories["setup_response_data"].return_value = ( + list(range(8)), + None, + ) + else: + mc = _make_master_config( + colocated=colocated, backend="megatron", megatron_enabled=True + ) + mc.async_rl.recompute_kv_cache_after_weight_updates = True + tokenizer = MagicMock(pad_token_id=0) + reserved_url = "http://10.0.0.1:5555/v1" + served_url = ( + "http://10.0.0.9:7/v1" + if scenario == "gym_served_mismatch" + else reserved_url ) + port_holder = MagicMock(name="port_holder") + fake_gym_actor = MagicMock(name="nemo_gym_actor") with ( - patch.object(sc_setup_mod, "_should_use_nemo_gym", return_value=True), - patch.object(sc_setup_mod, "spinup_nemo_gym_actor") as mock_spinup, - pytest.raises(NotImplementedError, match="vllm"), + patch.object(sc_setup_mod, "_should_use_nemo_gym", return_value=gym), + patch.object( + sc_setup_mod, "spinup_nemo_gym_actor", return_value=fake_gym_actor + ) as mock_spinup, + patch.object(sc_setup_mod, "router_replay_enabled", return_value=False), + patch.object(sc_setup_mod, "MegatronGeneration") as mock_megatron, + patch.object(sc_setup_mod, "ray") as mock_ray, ): - setup_single_controller(mc, MagicMock(pad_token_id=0)) - mock_spinup.assert_not_called() + mock_megatron.reserve_http_server_address.return_value = ( + reserved_url, + 5555, + port_holder, + ) + # Wire the real check through the class mock so the + # served-vs-reserved legs exercise the genuine logic. + mock_megatron.verify_served_address = ( + MegatronGeneration.verify_served_address + ) + mock_megatron.return_value.dp_openai_server_base_urls = [served_url] + if error_match is None: + actor_args, metrics = setup_single_controller(mc, tokenizer) + else: + with pytest.raises(RuntimeError, match=error_match): + setup_single_controller(mc, tokenizer) + + train_cluster = patched_factories["_build_clusters"].return_value[0] + inference_cluster = patched_factories["_build_clusters"].return_value[1] + # The megatron path never uses the generic generation factory and applies + # its config overrides before any build (_build_generation normally sets + # model_name; the kv-cache mode comes from the async_rl flag). + patched_factories["_build_generation"].assert_not_called() + assert mc.policy["generation"]["model_name"] == "test-model" + mcore_cfg = mc.policy["generation"]["mcore_generation_config"] + assert mcore_cfg["kv_cache_management_mode"] == "recompute" + assert mc.async_rl.recompute_kv_cache_after_weight_updates is False + # Reservation + holder lifecycle exist on the gym legs only. + if gym: + mock_megatron.reserve_http_server_address.assert_called_once_with( + train_cluster if colocated else inference_cluster, + mc.policy, + ) + mock_ray.kill.assert_called_once_with(port_holder) + else: + mock_megatron.reserve_http_server_address.assert_not_called() + mock_ray.kill.assert_not_called() + + # Construction: trainer first, then generation per mode — colocated + # wraps the trainer's policy and hands the reserved port to the trainer + # build; non-colocated builds on the dedicated cluster with the weight + # load skipped and the reserved port adopted by the engine (gym) or + # absent (native). + patched_factories["_build_trainer"].assert_called_once() + _, trainer_kwargs = patched_factories["_build_trainer"].call_args + assert trainer_kwargs["reserved_http_server_port"] == ( + 5555 if colocated and gym else None + ) + mock_megatron.assert_called_once_with( + config=mc.policy, + tokenizer=tokenizer, + cluster=None if colocated else inference_cluster, + policy=patched_factories["fake_policy"] if colocated else None, + processor=None, + weights_path=None, + skip_weight_load=not colocated, + reserved_http_server_port=5555 if gym and not colocated else None, + ) + if gym: + # Gym spins up on the reserved URL, before the served-address + # cross-check — so the mismatch leg sees it too. + _, spinup_kwargs = mock_spinup.call_args + assert spinup_kwargs["base_urls"] == [reserved_url] + else: + mock_spinup.assert_not_called() + if scenario == "gym_served_mismatch": + return # raised at the cross-check; no actor_args/metrics exist + + assert actor_args.gen_handle is mock_megatron.return_value + assert actor_args.trainer_handle is patched_factories["fake_policy"] + assert metrics.generation_init_time_s is not None + assert metrics.policy_init_time_s is not None + _, factory_kwargs = patched_factories["create_weight_synchronizer"].call_args + assert factory_kwargs["generation_backend"] == "megatron" + assert factory_kwargs["colocated"] is colocated + assert factory_kwargs["inference_cluster"] is inference_cluster + if gym: + assert actor_args.env_handles["nemo_gym"] is fake_gym_actor + assert metrics.nemo_gym_init_time_s is not None + assert metrics.generation_init_reserve_time_s is not None + else: + # Reserve/load split is populated on the gym-on path only. + assert metrics.generation_init_reserve_time_s is None class TestNativeTQRecoverySetup: diff --git a/tools/refit_verifier.py b/tools/refit_verifier.py index 4a0d4cb0841..f48ef4d54f1 100644 --- a/tools/refit_verifier.py +++ b/tools/refit_verifier.py @@ -610,7 +610,6 @@ def init_sglang(inference_cluster, generation_config): def initialize_generation_with_policy( init_generation_fn: Callable, init_policy_fn: Callable, - init_time_key: str, colocated_inference: bool, worker_init_timing_metrics: dict, policy: Policy | None = None, @@ -625,7 +624,6 @@ def initialize_generation_with_policy( Args: init_generation_fn: Callable returning (engine, init_time_s). init_policy_fn: Callable returning (policy, init_time_s). - init_time_key: Metrics key for generation init time. colocated_inference: Whether inference is colocated with training. worker_init_timing_metrics: Dict populated with init/parallel timing. policy: Optional pre-initialized policy; if set, init_policy_fn is skipped. @@ -648,7 +646,7 @@ def initialize_generation_with_policy( policy, policy_time = policy_future.result() parallel_wall_time = time.perf_counter() - parallel_start_time - worker_init_timing_metrics[init_time_key] = generation_time + worker_init_timing_metrics["generation_init_time_s"] = generation_time worker_init_timing_metrics["policy_init_time_s"] = policy_time worker_init_timing_metrics["parallel_wall_time_s"] = parallel_wall_time worker_init_timing_metrics["parallel_init_enabled"] = 1.0 @@ -658,7 +656,7 @@ def initialize_generation_with_policy( flush=True, ) policy_generation, generation_time = init_generation_fn() - worker_init_timing_metrics[init_time_key] = generation_time + worker_init_timing_metrics["generation_init_time_s"] = generation_time if policy is None: policy, policy_time = init_policy_fn() @@ -765,7 +763,6 @@ def init_policy_fn(): policy_generation, _ = initialize_generation_with_policy( init_generation_fn=lambda: init_sglang(cluster, generation_config), init_policy_fn=init_policy_fn, - init_time_key="sglang_init_time_s", colocated_inference=True, worker_init_timing_metrics=worker_init_timing_metrics, )