Repository navigation
fix ODF logging and add min_abs_adv #2140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
ce67b67
30746d4
51c56a8
23862e1
d439ad5
d1c8b85
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import warnings | ||
| from pathlib import Path | ||
| from typing import Annotated, Any, Literal, TypeAlias | ||
|
|
||
|
|
@@ -514,10 +515,20 @@ class BufferConfig(BaseConfig): | |
| ), | ||
| ] = 0.0 | ||
|
|
||
| min_abs_adv: Annotated[ | ||
| float | None, | ||
| Field( | ||
| description=( | ||
| "Only send rollouts with absolute advantage greater than this threshold to training. " | ||
| "Rollouts are still kept for orchestrator logging. If None, no advantage-based filtering is applied." | ||
| ), | ||
| ), | ||
| ] = None | ||
|
cursor[bot] marked this conversation as resolved.
faresobeid marked this conversation as resolved.
|
||
|
|
||
| online_difficulty_filtering: Annotated[ | ||
| bool, | ||
| Field( | ||
| description="Whether to filter rollouts based on difficulty. If True, rollouts with average reward 0.0 or 1.0 are not added to the buffer.", | ||
| description="Deprecated alias for setting min_abs_adv=0.0.", | ||
| ), | ||
| ] = False | ||
|
|
||
|
|
@@ -541,6 +552,30 @@ def validate_env_ratios(self): | |
| assert all(ratio > 0 for ratio in self.env_ratios), "All env_ratios must be positive." | ||
| return self | ||
|
|
||
| @model_validator(mode="after") | ||
| def resolve_min_abs_adv(self): | ||
| if not self.online_difficulty_filtering: | ||
| return self | ||
|
|
||
| warnings.warn( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we use prime-rl logger?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm not sure, also config validation happens before the prime-rl logger is set up
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we fixed this so that |
||
| "buffer.online_difficulty_filtering is deprecated and will be removed in a future version. " | ||
| "Use buffer.min_abs_adv = 0.0 instead.", | ||
| FutureWarning, | ||
| stacklevel=2, | ||
| ) | ||
|
|
||
| if self.min_abs_adv is None: | ||
| self.min_abs_adv = 0.0 | ||
| return self | ||
|
faresobeid marked this conversation as resolved.
|
||
|
|
||
| if self.min_abs_adv != 0.0: | ||
| raise ValueError( | ||
| "buffer.online_difficulty_filtering is a deprecated alias for buffer.min_abs_adv=0.0 and " | ||
| "cannot be combined with a different buffer.min_abs_adv value." | ||
| ) | ||
|
|
||
| return self | ||
|
|
||
|
|
||
| class VerificationConfig(BaseConfig): | ||
| """Configures rollout verification and rubric scoring.""" | ||
|
|
@@ -979,9 +1014,9 @@ def validate_verification_config(self): | |
| if self.verification.enabled: | ||
| return self | ||
|
|
||
| if self.buffer.online_difficulty_filtering: | ||
| if self.buffer.min_abs_adv is not None: | ||
| raise ValueError( | ||
| "verification.enabled cannot be False when buffer.online_difficulty_filtering is True. " | ||
| "verification.enabled cannot be False when buffer.min_abs_adv is set. " | ||
| "These features depend on rewards which are disabled when verification.enabled=False." | ||
| ) | ||
| if self.buffer.easy_threshold is not None: | ||
|
|
@@ -997,10 +1032,10 @@ def validate_verification_config(self): | |
| return self | ||
|
|
||
| @model_validator(mode="after") | ||
| def validate_length_shaping_requires_online_difficulty_filtering(self): | ||
| def validate_length_shaping_requires_min_abs_adv(self): | ||
| if isinstance(self.advantage, DefaultAdvantageConfig) and self.advantage.length_shaping_alpha is not None: | ||
| if not self.buffer.online_difficulty_filtering: | ||
| raise ValueError("Group Relative Reward (GR³) scaling requires online difficulty filtering") | ||
| if self.buffer.min_abs_adv is None: | ||
| raise ValueError("Group Relative Reward (GR³) scaling requires buffer.min_abs_adv to be set") | ||
| return self | ||
|
|
||
| @model_validator(mode="after") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,7 @@ | |
|
|
||
| import tomli_w | ||
|
|
||
| from prime_rl.orchestrator.advantage import compute_advantages | ||
| from prime_rl.orchestrator.advantage import compute_advantages, compute_keep_mask | ||
| from prime_rl.orchestrator.eval_utils import compute_eval_ckpt_step, get_eval_sampling_args | ||
| from prime_rl.orchestrator.event_loop_lag import EventLoopLagMonitor | ||
| from prime_rl.orchestrator.patches import monkey_patch_chat_completion_logprobs, monkey_patch_oai_iterable_types | ||
|
|
@@ -532,7 +532,7 @@ def _cleanup_env_processes(): | |
| # Await train rollouts | ||
| await train_task | ||
| generate_completions_time = scheduler.last_batch_generation_time | ||
| train_rollouts = train_task.result() | ||
| train_rollouts, selected_rollout_mask = train_task.result() | ||
|
|
||
| # VLM: offload base64 images to disk immediately to free memory | ||
| if is_vlm: | ||
|
|
@@ -553,11 +553,12 @@ def _cleanup_env_processes(): | |
| rewards = [r["reward"] for r in train_rollouts] | ||
| completion_lens = [get_completion_len(r) for r in train_rollouts] | ||
| advantages = compute_advantages( | ||
| rewards, | ||
| completion_lens, | ||
| config.rollouts_per_example, | ||
| config.advantage, | ||
| rewards=rewards, | ||
|
faresobeid marked this conversation as resolved.
|
||
| completion_lengths=completion_lens, | ||
| samples_per_problem=config.rollouts_per_example, | ||
| advantage_config=config.advantage, | ||
| ) | ||
| rollout_keep_mask = compute_keep_mask(advantages, config.buffer.min_abs_adv) | ||
|
|
||
| # Convert rollouts to training samples | ||
| parallel_preprocess_start = time.perf_counter() | ||
|
|
@@ -598,7 +599,9 @@ def process_rollout(rollout: vf.RolloutOutput, rollout_idx: int) -> list[Trainin | |
| rollout_samples_per_rollout: list[int] = [] | ||
| num_prefill_tokens = 0 | ||
| num_decode_tokens = 0 | ||
| for rollout, advantage, samples in zip(train_rollouts, advantages, results): | ||
| for rollout, advantage, selected_for_training, samples in zip( | ||
| train_rollouts, advantages, selected_rollout_mask, results | ||
| ): | ||
| rollout_prefill_tokens = 0 | ||
| rollout_decode_tokens = 0 | ||
| if samples is not None: | ||
|
|
@@ -610,18 +613,28 @@ def process_rollout(rollout: vf.RolloutOutput, rollout_idx: int) -> list[Trainin | |
| sample_prefill_tokens = len(sample.prompt_ids) + len(sample.completion_mask) - sample_decode_tokens | ||
| rollout_decode_tokens += sample_decode_tokens | ||
| rollout_prefill_tokens += sample_prefill_tokens | ||
| train_examples.append(sample) | ||
| if selected_for_training: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh but this doesn't work, no? bc now all rollouts from the scheduler might get filtered and we get an empty batch? imo, the filtering should happen inside the scheduler so that we are guaranteed to get the specified number of training rollouts |
||
| train_examples.append(sample) | ||
| else: | ||
| rollout_samples_per_rollout.append(0) | ||
| rollout_prefill_lens.append(rollout_prefill_tokens) | ||
| rollout_decode_lens.append(rollout_decode_tokens) | ||
| num_prefill_tokens += rollout_prefill_tokens | ||
| num_decode_tokens += rollout_decode_tokens | ||
|
|
||
| num_filtered_rollouts = sum(not keep_rollout for keep_rollout in rollout_keep_mask) | ||
| parallel_preprocess_time = time.perf_counter() - parallel_preprocess_start | ||
| logger.debug( | ||
| f"Converted {len(train_rollouts)} rollouts ({num_unique_examples} unique examples) " | ||
| f"to {len(train_examples)} training examples" | ||
| f"to {len(train_examples)} training examples ({sum(selected_rollout_mask)} selected rollouts)" | ||
| + ( | ||
| "" | ||
| if config.buffer.min_abs_adv is None | ||
| else ( | ||
| f" after filtering {num_filtered_rollouts} rollout(s) with " | ||
| f"|advantage| <= {config.buffer.min_abs_adv}" | ||
| ) | ||
| ) | ||
| ) | ||
|
|
||
| # Compute teacher logprobs if teacher model is configured | ||
|
|
@@ -658,6 +671,7 @@ def process_rollout(rollout: vf.RolloutOutput, rollout_idx: int) -> list[Trainin | |
| "example_id": [rollout["example_id"] for rollout in train_rollouts], | ||
| "task": [rollout["task"] for rollout in train_rollouts], | ||
| "reward": [rollout["reward"] for rollout in train_rollouts], | ||
| "passes_min_abs_adv": rollout_keep_mask, | ||
| "is_truncated": [rollout["is_truncated"] for rollout in train_rollouts], | ||
| "stop_condition": [rollout.get("stop_condition") for rollout in train_rollouts], | ||
| "seq_len": [get_seq_len(rollout) for rollout in train_rollouts], | ||
|
|
@@ -754,6 +768,7 @@ def compute_solve_rates(df): | |
| "solve_none/all": solve_none, | ||
| "solve_all/all": solve_all, | ||
| "effective_batch_size/all": effective_batch_size, | ||
| "filtered_rollouts/all": num_filtered_rollouts / max(num_rollouts, 1), | ||
| **{f"batch/{env}": r for env, r in results_df.task.value_counts(normalize=True).items()}, | ||
| # Time metrics | ||
| "time/step": step_time, | ||
|
|
@@ -794,6 +809,7 @@ def compute_solve_rates(df): | |
| to_log[f"reward/{env}/mean"] = env_by_example.reward.mean().mean() | ||
| to_log[f"reward/{env}/max"] = env_by_example.reward.mean().max() | ||
| to_log[f"reward/{env}/min"] = env_by_example.reward.mean().min() | ||
| to_log[f"filtered_rollouts/{env}"] = 1.0 - env_df.passes_min_abs_adv.mean() | ||
| solve_none, solve_all, effective_batch_size = compute_solve_rates(env_df) | ||
| to_log[f"solve_none/{env}"] = solve_none | ||
| to_log[f"solve_all/{env}"] = solve_all | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.