Rebranding Policy Implementation as a Generator Implementation

[Jack-Khuu]

Everything boils down to these simple changes

• policy.py => generator.py
• Policy => Generator

But why are there still mentions of policy in the repo?

That's because this PR intentionally does NOT change the assigned variables themselves.
A Policy is the concrete concept, and in this case it's just backed by a particular implementation. The yaml files are untouched for the same reason (it's configuring the Policy, which happens to be this Generator under the hood)

policy: Generator = Generator(...)

The Generator class has no mentions of policy because it's just a vllm generator (it can be used in other components like a Judge)

But what if we want other Generator implementations?

Great, we can rename this class when we actually need to (VllmGenerator, Vllm doesn't sound terrible)
- Update: From the comments, another idea would be to make Generator the factory spawning the implementations (vllm, sglang)

"I don't like the name"

Generator > Policy, so a different name is not a blocker.
If anyone gets more than 3 votes on a different name, we'll rename

---

wandb: torchforge/grpo-training/runs/aoypk2tk

[allenwang28]

not a hill I'm going to die on, but if we wanted to introduce a Judge that re-uses our vLLM implementation, then we would do this:

class Judge(Generator):
    ...

does that make sense to others? Is there a world where we have a vLLM base actor and we have Generator, Judge, RewardModel based on that?

[felipemello1]

regarding VllmGenerator . Is it realistic that someone in the community at some point will contribute with a SGLangGenerator?

Not for this PR, but if/when this happens, it makes sense to me to keep everything as Generator, untouched, and have it as a factory for vLLMGenerator, SGLangGenerator, etc.

[allenwang28]

i'm not sure we should support any and all generators in Forge, but my point was slightly different. Does it make sense logically for a Judge to inherit from a Generator? It doesn't really to me - it makes more sense if they all inherit from some InferenceEngine or something like that

[Jack-Khuu]

if we wanted to introduce a Judge that re-uses our vLLM implementation, then we would do this: class Judge(Generator):

Would we subclass? This might be a different question, but I thought forge/tune were intentionally avoiding subclassing
I would think Judge is

class Judge(ForgeActor):
  generator: GeneratorInterface  # Member (whether it be an instance ServiceInterface, self instantiated generator, etc.)

Edit: Discussed offline. We're gonna try sublassing

[Jack-Khuu]

Update (10/9):

• Delta since last is [just a rebase] (tm)
• Regarding relation to other classes: class Judge(Generator) actually doesn't seem terribly bizarre at this time
  • Not in this PR, but we might even consider pushing it further with moving weight updates to Policy(Generator)

---

Is there a world where we have a vLLM base actor and we have Generator, Judge, RewardModel based on that

it makes sense to me to keep everything as Generator, untouched, and have it as a factory for vLLMGenerator, SGLangGenerator, etc.

Generator can definitely be converted to a factory under the hood when we need to support them (we could even have the generated instance from the factory be a vLLM base actor when we start offering different generation options

[codecov-commenter]

Codecov Report

❌ Patch coverage is 63.71681% with 41 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@399b20d). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
src/forge/actors/generator.py	20.00%	32 Missing ⚠️
src/forge/actors/__init__.py	0.00%	3 Missing ⚠️
tests/integration_tests/test_policy_update.py	0.00%	3 Missing ⚠️
tests/unit_tests/test_generator_config.py	95.52%	3 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #302   +/-   ##
=======================================
  Coverage        ?   73.68%           
=======================================
  Files           ?       81           
  Lines           ?     7732           
  Branches        ?        0           
=======================================
  Hits            ?     5697           
  Misses          ?     2035           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[pbontrager]

Thanks for this! One small request but otherwise thanks!