Skip to content

Commit ead709a

Browse files
committed
Address reviewer comments
Signed-off-by: Teodor-Dumitru Ene <teodord.ene@gmail.com>
1 parent 492b966 commit ead709a

2 files changed

Lines changed: 21 additions & 12 deletions

File tree

nemo_rl/algorithms/grpo.py

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,7 +1200,11 @@ def refit_policy_generation(
12001200
"""
12011201
if colocated_inference:
12021202
policy.offload_before_refit()
1203-
policy_generation.prepare_for_generation(tags=["weights"])
1203+
# Colocated inference needs to prepare for generation.
1204+
# Megatron non-colocated inference needs to prepare a different way:
1205+
# enter inference mode after refit.
1206+
if colocated_inference or isinstance(policy_generation, MegatronGeneration):
1207+
policy_generation.prepare_for_generation(tags=["weights"])
12041208

12051209
# Create a context manager that does nothing when timer is None
12061210
timer_context = (
@@ -1273,7 +1277,11 @@ def refit_policy_generation(
12731277

12741278
if colocated_inference:
12751279
policy.offload_after_refit()
1276-
policy_generation.prepare_for_generation(tags=["kv_cache"])
1280+
# Colocated inference needs to prepare for generation.
1281+
# Megatron non-colocated inference needs to prepare a different way:
1282+
# enter inference mode after refit.
1283+
if colocated_inference or isinstance(policy_generation, MegatronGeneration):
1284+
policy_generation.prepare_for_generation(tags=["kv_cache"])
12771285

12781286

12791287
def _log_mixed_rewards_and_advantages_information(
@@ -2548,6 +2556,10 @@ def async_grpo_train(
25482556
val_at_start = master_config.grpo["val_at_start"]
25492557
val_at_end = master_config.grpo["val_at_end"]
25502558
colocated_inference = master_config.policy["generation"]["colocated"]["enabled"]
2559+
# Megatron generation backend needs explicit suspend/resume around refits.
2560+
is_megatron_generation = (
2561+
master_config.policy["generation"]["backend"] == "megatron"
2562+
)
25512563

25522564
# Initialize advantage estimator
25532565
adv_estimator = _create_advantage_estimator(master_config)
@@ -2658,13 +2670,10 @@ def async_grpo_train(
26582670
if NEED_REFIT and POLICY_GENERATION_STALE:
26592671
print("🔄 Refitting policy generation with actual model weights...")
26602672
try:
2661-
is_megatron = (
2662-
master_config.policy["generation"]["backend"] == "megatron"
2663-
)
2664-
if is_megatron:
2673+
if is_megatron_generation:
26652674
policy_generation.suspend_for_refit(recompute_kv_cache=False)
26662675
refit_policy_generation(policy, policy_generation, colocated_inference)
2667-
if is_megatron:
2676+
if is_megatron_generation:
26682677
policy_generation.resume_after_refit(recompute_kv_cache=False)
26692678
print("✅ Policy generation refit completed successfully")
26702679
POLICY_GENERATION_STALE = False
@@ -3020,15 +3029,12 @@ def async_grpo_train(
30203029
# Pause the megatron engine loop before weight transfer.
30213030
# Without this, swap_weights_via_reshard races with in-flight
30223031
# CUDA-graph replays and corrupts state.
3023-
is_megatron = (
3024-
master_config.policy["generation"]["backend"] == "megatron"
3025-
)
30263032
recompute_kv_cache = (
30273033
master_config.grpo
30283034
.get("async_grpo", {})
30293035
.get("recompute_kv_cache_after_weight_updates", False)
30303036
)
3031-
if is_megatron:
3037+
if is_megatron_generation:
30323038
policy_generation.suspend_for_refit(
30333039
recompute_kv_cache=recompute_kv_cache
30343040
)
@@ -3041,7 +3047,7 @@ def async_grpo_train(
30413047
)
30423048
POLICY_GENERATION_STALE = False
30433049

3044-
if is_megatron:
3050+
if is_megatron_generation:
30453051
policy_generation.resume_after_refit(
30463052
recompute_kv_cache=recompute_kv_cache
30473053
)

nemo_rl/models/policy/workers/megatron_policy_worker.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,9 @@ def __init__(
220220
if cuda_graph_impl == "full_iteration" or (
221221
cuda_graph_impl == "local" and scope_has_full_iteration
222222
):
223+
warnings.warn(
224+
"Disabling check_for_nan_in_loss: full-iteration CUDA graph cannot be interrupted."
225+
)
223226
self.megatron_cfg.rerun_state_machine.check_for_nan_in_loss = False
224227

225228
# Validate configuration

0 commit comments

Comments
 (0)