-
Notifications
You must be signed in to change notification settings - Fork 929
[RL] - MessageEnv, Rollout types, Rubric, Renderer #3453
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 24 commits
6913120
7c7c9f7
8e4fc60
dc690a1
40ef4c5
767022f
50b5572
86ed252
31dcd8b
7dad6f5
d440e46
1523db5
312c292
14aaeca
6292f1d
e9af27a
85880db
9243ba1
701b4d1
11003ba
5044e86
2dcd0f9
7a72a41
0cf8460
df376bb
d3ba2df
de7c887
c2efd2b
8b597ba
79c3903
621e3cf
3c69ae6
c77d47f
75e69d4
80104f5
4bf9b33
9d91d4c
d33903e
89f8f85
04c47c2
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -163,6 +163,9 @@ class SamplingConfig: | |||||
| max_tokens: int = 100 | ||||||
| """Maximum number of tokens to generate per completion.""" | ||||||
|
|
||||||
| stop_token_ids: list[int] = field(default_factory=list) | ||||||
| """Role-boundary stop tokens from the renderer (e.g. Qwen3 `<|im_end|>`).""" | ||||||
|
|
||||||
|
|
||||||
| class VLLMGenerator(Actor, Configurable): | ||||||
| """ | ||||||
|
|
@@ -411,6 +414,7 @@ async def generate( | |||||
| top_p=_sampling_config.top_p, | ||||||
| max_tokens=_sampling_config.max_tokens, | ||||||
| n=_sampling_config.n, | ||||||
| stop_token_ids=list(_sampling_config.stop_token_ids) or None, | ||||||
|
Contributor
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.
Suggested change
|
||||||
| seed=self.config.debug.seed, | ||||||
| logprobs=1, | ||||||
| output_kind=RequestOutputKind.FINAL_ONLY, | ||||||
|
|
@@ -457,7 +461,6 @@ async def generate( | |||||
| Completion( | ||||||
| policy_version=self.policy_version, | ||||||
| prompt_idx=prompt_idx, | ||||||
| text=sample.text, | ||||||
| token_ids=sample.token_ids, | ||||||
| token_logprobs=per_token_logprobs, | ||||||
| finish_reason=sample.finish_reason, | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,7 +25,8 @@ | |
| from torchtitan.experiments.rl.batcher import BatchConfig, Batcher | ||
| from torchtitan.experiments.rl.grpo import GRPOLoss, RLTrainer | ||
| from torchtitan.experiments.rl.observability.metrics import MetricsProcessor | ||
| from torchtitan.experiments.rl.sum_digits import SumDigitsEnv | ||
| from torchtitan.experiments.rl.renderer import RendererConfig | ||
| from torchtitan.experiments.rl.tasks.sum_digits import SumDigitsDataset, SumDigitsTask | ||
| from torchtitan.models.qwen3 import model_registry | ||
|
|
||
|
|
||
|
|
@@ -39,10 +40,10 @@ def rl_grpo_qwen3_0_6b() -> RLTrainer.Config: | |
| num_prompts_per_step=5, | ||
| num_validation_samples=20, | ||
| compile=CompileConfig(enable=True, backend="aot_eager"), | ||
| env=SumDigitsEnv.Config(seed=42, correctness_reward=1.0, format_reward=0.3), | ||
| validation_env=SumDigitsEnv.Config( | ||
| seed=99, correctness_reward=1.0, format_reward=0.3 | ||
| ), | ||
| train_dataset=SumDigitsDataset.Config(seed=42), | ||
| validation_dataset=SumDigitsDataset.Config(seed=99), | ||
|
Contributor
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. I was thinking should we change validation sounds more like supervised learning
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. no strong opinions. I dont mind changing. But i think that we should use the same terminology across the codebase. |
||
| tasks={"sum_digits": SumDigitsTask.Config()}, | ||
| renderer=RendererConfig(name="qwen3", enable_thinking=True), | ||
|
Contributor
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. Why render related to a model name? Is it because it will use tokenizer path? Currently
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.
|
||
| metrics=MetricsProcessor.Config(enable_wandb=True), | ||
| batcher=Batcher.Config( | ||
| batch=BatchConfig(local_batch_size=2, global_batch_size=8, seq_len=2048), | ||
|
|
@@ -80,7 +81,7 @@ def rl_grpo_qwen3_0_6b() -> RLTrainer.Config: | |
| n=group_size, | ||
| temperature=0.8, | ||
| top_p=0.95, | ||
| max_tokens=100, | ||
| max_tokens=700, | ||
| ), | ||
| ), | ||
| ) | ||
|
|
@@ -96,10 +97,10 @@ def rl_grpo_qwen3_1_7b() -> RLTrainer.Config: | |
| num_prompts_per_step=5, | ||
| num_validation_samples=20, | ||
| compile=CompileConfig(enable=True, backend="aot_eager"), | ||
| env=SumDigitsEnv.Config(seed=42, correctness_reward=1.0, format_reward=0.3), | ||
| validation_env=SumDigitsEnv.Config( | ||
| seed=99, correctness_reward=1.0, format_reward=0.3 | ||
| ), | ||
| train_dataset=SumDigitsDataset.Config(seed=42), | ||
| validation_dataset=SumDigitsDataset.Config(seed=99), | ||
| tasks={"sum_digits": SumDigitsTask.Config()}, | ||
| renderer=RendererConfig(name="qwen3", enable_thinking=True), | ||
| metrics=MetricsProcessor.Config(enable_wandb=True), | ||
| batcher=Batcher.Config( | ||
| batch=BatchConfig(local_batch_size=2, global_batch_size=8, seq_len=2048), | ||
|
|
@@ -138,7 +139,7 @@ def rl_grpo_qwen3_1_7b() -> RLTrainer.Config: | |
| n=group_size, | ||
| temperature=0.8, | ||
| top_p=0.95, | ||
| max_tokens=100, | ||
| max_tokens=700, | ||
|
Contributor
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. Will need to change batcher max_seq_length accordingly, otherwise all the samples will be dropped because they are longer than |
||
| ), | ||
| ), | ||
| ) | ||
|
|
@@ -154,10 +155,10 @@ def rl_grpo_qwen3_14b() -> RLTrainer.Config: | |
| num_prompts_per_step=5, | ||
| num_validation_samples=20, | ||
| compile=CompileConfig(enable=True, backend="aot_eager"), | ||
| env=SumDigitsEnv.Config(seed=42, correctness_reward=1.0, format_reward=0.3), | ||
| validation_env=SumDigitsEnv.Config( | ||
| seed=99, correctness_reward=1.0, format_reward=0.3 | ||
| ), | ||
| train_dataset=SumDigitsDataset.Config(seed=42), | ||
| validation_dataset=SumDigitsDataset.Config(seed=99), | ||
| tasks={"sum_digits": SumDigitsTask.Config()}, | ||
| renderer=RendererConfig(name="qwen3", enable_thinking=True), | ||
| metrics=MetricsProcessor.Config(enable_wandb=True), | ||
| batcher=Batcher.Config( | ||
| batch=BatchConfig(local_batch_size=2, global_batch_size=8, seq_len=2048), | ||
|
|
@@ -195,14 +196,14 @@ def rl_grpo_qwen3_14b() -> RLTrainer.Config: | |
| n=group_size, | ||
| temperature=0.8, | ||
| top_p=0.95, | ||
| max_tokens=100, | ||
| max_tokens=700, | ||
| ), | ||
| ), | ||
| ) | ||
|
|
||
|
|
||
| def rl_grpo_qwen3_0_6b_batch_invariant() -> RLTrainer.Config: | ||
| """On-policy GRPO config for Qwen3-0.6B under same parallelism (4 GPUs: 2 gen + 2 train). | ||
| """On-policy GRPO config for Qwen3-0.6B (4 GPUs: 2 gen + 2 train). | ||
|
|
||
| Enables deterministic + batch-invariant mode for true on-policy RL training. | ||
| """ | ||
|
|
@@ -215,10 +216,10 @@ def rl_grpo_qwen3_0_6b_batch_invariant() -> RLTrainer.Config: | |
| num_prompts_per_step=5, | ||
| num_validation_samples=20, | ||
| compile=CompileConfig(enable=True, backend="aot_eager"), | ||
| env=SumDigitsEnv.Config(seed=42, correctness_reward=1.0, format_reward=0.3), | ||
| validation_env=SumDigitsEnv.Config( | ||
| seed=99, correctness_reward=1.0, format_reward=0.3 | ||
| ), | ||
| train_dataset=SumDigitsDataset.Config(seed=42), | ||
| validation_dataset=SumDigitsDataset.Config(seed=99), | ||
| tasks={"sum_digits": SumDigitsTask.Config()}, | ||
| renderer=RendererConfig(name="qwen3", enable_thinking=True), | ||
| metrics=MetricsProcessor.Config(enable_wandb=True), | ||
| batcher=Batcher.Config( | ||
| batch=BatchConfig(local_batch_size=2, global_batch_size=8, seq_len=2048), | ||
|
|
@@ -260,7 +261,7 @@ def rl_grpo_qwen3_0_6b_batch_invariant() -> RLTrainer.Config: | |
| n=group_size, | ||
| temperature=0.8, | ||
| top_p=0.95, | ||
| max_tokens=100, | ||
| max_tokens=700, | ||
| ), | ||
| debug=batch_invariant_config, | ||
| ), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| # Copyright (c) Meta Platforms, Inc. and affiliates. | ||
| # All rights reserved. | ||
| # | ||
| # This source code is licensed under the BSD-style license found in the | ||
| # LICENSE file in the root directory of this source tree. | ||
|
|
||
| from torchtitan.experiments.rl.env_types.message_env import ( | ||
| MessageEnv, | ||
| ResetOutput, | ||
| StepOutput, | ||
| ) | ||
| from torchtitan.experiments.rl.env_types.renderer_env import ( | ||
| RendererEnv, | ||
| RendererEnvConfig, | ||
| TokenizedStepOutput, | ||
| ) | ||
|
|
||
| __all__ = [ | ||
| "RendererEnvConfig", | ||
| "MessageEnv", | ||
| "ResetOutput", | ||
| "StepOutput", | ||
| "RendererEnv", | ||
| "TokenizedStepOutput", | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| # Copyright (c) Meta Platforms, Inc. and affiliates. | ||
| # All rights reserved. | ||
| # | ||
| # This source code is licensed under the BSD-style license found in the | ||
| # LICENSE file in the root directory of this source tree. | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import abc | ||
| from dataclasses import dataclass, field | ||
|
|
||
| from renderers import Message, ToolSpec | ||
|
|
||
|
|
||
| @dataclass(kw_only=True, slots=True) | ||
| class ResetOutput: | ||
|
Contributor
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. can this be special subclass of StepOutput?
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. I was also a bit unhappy with these classes, but i didnt really want to subclass. Maybe i could have the StepOutput be a field in it. Let me check a few options. |
||
| """Initial messages + tool specs from `MessageEnv.reset`.""" | ||
|
|
||
| messages: list[Message] # [M_initial] | ||
| """The set of messages that form an initial prompt for the model.""" | ||
|
|
||
| tools: list[ToolSpec] = field(default_factory=list) # [K_tools] | ||
| """Tool schemas exposed to the model. Empty for tool-less envs.""" | ||
|
|
||
|
|
||
| @dataclass(kw_only=True, slots=True) | ||
| class StepOutput: | ||
| """Env response to one parsed assistant message. | ||
|
|
||
| `done` ends the conversation; `RendererEnv` maps it to a `RolloutStatus` | ||
| and detects truncation/errors. The env may attach `reward_components` | ||
| (e.g. tool-call success); the rubric decides how to use them. | ||
| """ | ||
|
|
||
| messages: list[Message] = field(default_factory=list) # [M_env] | ||
|
Contributor
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. noob question: why it's a list (instead of this step's single message)?
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. The prompt can start from an ongoing conversation, for example. But, a simple case it: ["system_msg", "user_message"]. Thats already two.
Contributor
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. let's docstring this |
||
| """Env-appended messages (tool / user replies). Empty when the rollout | ||
| terminates with no follow-up.""" | ||
|
|
||
| done: bool = False | ||
| """`True` ends the rollout.""" | ||
|
|
||
| reward_components: dict[str, float] = field(default_factory=dict) | ||
|
Contributor
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.
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. i think that rewards is also not obvious. Let me think of a few other options. |
||
| """Optional reward signal the env provides for this step; the rubric decides | ||
| whether and how to use it. Empty if the env scores nothing.""" | ||
|
|
||
| def __post_init__(self) -> None: | ||
| # env replies are tool/user turns; the assistant turn comes from the model | ||
| if any(m.get("role") == "assistant" for m in self.messages): | ||
| raise ValueError("StepOutput.messages may not contain assistant messages") | ||
|
|
||
|
|
||
| class MessageEnv(abc.ABC): | ||
| """User-written env in message space. Implement `reset` + `step_message`; | ||
| `RendererEnv` wraps it with a `Renderer` for tokens. | ||
|
Contributor
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. what does "a Renderer for tokens" mean?
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. MessageEnv operate in messages, i.e. strings. No tokens. I can rewrite to make it more obvious
Contributor
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.
?
sounds confusing when you say "RendererEnv takes messages from envs"
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. transforms into messages** haha
why? Not real APIs or precise flow -- just sharing the main idea |
||
|
|
||
| Example: | ||
|
|
||
| class SumDigitsEnv(MessageEnv): | ||
| async def reset(self) -> ResetOutput: | ||
| return ResetOutput(messages=[{"role": "user", "content": "sum [1, 2]"}]) | ||
|
|
||
| async def step_message(self, msg: Message) -> StepOutput: | ||
| return StepOutput(done=True) # single-turn; rubric scores later | ||
| """ | ||
|
|
||
| @abc.abstractmethod | ||
| async def reset(self) -> ResetOutput: | ||
| """Return the initial conversation + tools for prompt rendering.""" | ||
|
|
||
| @abc.abstractmethod | ||
| async def step_message(self, msg: Message) -> StepOutput: | ||
|
Contributor
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. function signature could be improved
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. "is msg always from assistant?" "MessageEnv.step_message(msg) " |
||
| """Return the env's reply to an assistant message. | ||
|
|
||
| `RendererEnv` handles finish_reason / length / parse failures before | ||
| calling this, so the env only sees a successfully parsed message. | ||
|
|
||
| Args: | ||
| msg: Parsed assistant message (`content`, optional | ||
| `reasoning_content`, optional `tool_calls`). | ||
|
|
||
| Returns: | ||
| `StepOutput` with env reply messages. | ||
| """ | ||
|
|
||
| async def close(self) -> None: | ||
| """Release env-owned resources. Default no-op; idempotent.""" | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would this be used for sft as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to depend on renders lastest main?