Add YAML-based configuration support for vLLM main #116

DNXie · 2025-09-02T19:57:53Z

Support vllm/main to take configs from yaml file
Support grpo/main to take configs from yaml file (Removed from this PR, in Add YAML config file for grpo.main #141)
Remove PolicyConfig
Make WorkerConfig inherit EngineArgs and added from_dict to WorkerConfig.
Rename WorkerConfig to EngineConfig
Rename SamplingOverrides to SamplingConfig
Updated Policy __post_init__ to use from_dict when a dict is passed.
Added unit tests for config file reading.

Test Run vllm/main:

export HF_HUB_DISABLE_XET=1
python -m apps.vllm.main --config apps/vllm/config.yaml

Requesting generation...

Generation Results:
================================================================================
Sample 1:
User: Tell me a joke
Assistant: . I need a laugh.
Here's one: A man walked into a library
--------------------------------------------------------------------------------
Sample 2:
User: Tell me a joke
Assistant:
I'll try to come up with one. Why did the scarecrow win
--------------------------------------------------------------------------------

Shutting down..

Test Run grpo/main:

python -m apps.grpo.main --config apps/grpo/llama3_8b.yaml

...
Generated 10 rollouts w/ average reward 0.0
Generated 20 rollouts w/ average reward 0.0
Generated 30 rollouts w/ average reward 0.1
Generated 40 rollouts w/ average reward 0.1
Generated 50 rollouts w/ average reward 0.0
Completed 10 training steps
Latest loss: 114.34945392608643
Generated 60 rollouts w/ average reward 0.0
Generated 70 rollouts w/ average reward 0.0
Generated 80 rollouts w/ average reward 0.0
Generated 90 rollouts w/ average reward 0.0
...

Unit test

python forge/tests/unit_tests/test_policy_config.py

Monkey patched Triton's _build! See /home/dxie/.fbpkg_conda_envs/forge-af45115/lib/python3.10/site-packages/patch_triton.py
Monkey patched Triton's nvsmi! See /home/dxie/.fbpkg_conda_envs/forge-af45115/lib/python3.10/site-packages/patch_triton.py
INFO 09-02 20:38:19 [__init__.py:235] Automatically detected platform cuda.
....
----------------------------------------------------------------------
Ran 4 tests in 0.002s

OK

Test `vllm_args`

I tested with vllm_args

string (see test case test_invalid_worker_config_from_dict)
null (see the config file)
with parameters values (see test case test_policy_yaml_config_loading)

pbontrager

Left some comments. Also remember to update any other app that uses Policy after making the policy changes

apps/vllm/config.yaml

apps/vllm/main.py

src/forge/actors/policy.py

apps/vllm/main.py

pbontrager

Thanks for doing this, I left some comments on config stricture but this should be good.

.github/workflows/unit_test.yaml

pbontrager · 2025-09-08T17:58:49Z

apps/grpo/llama3_8b.yaml

These should be organized by service name:

trainer: model: dataset: ... service: # for service config policy: ... service: replay_buffer: ... service ...

But also I'd leave grpo for a followup PR

Removed for now. Will open a followup PR

Submitted the followup PR here #141

apps/vllm/llama3_8b.yaml

src/forge/actors/policy.py

pbontrager · 2025-09-08T18:05:18Z

src/forge/actors/policy.py

You need to pull changes from main here, maybe this can inherit from GuidedDecoding in vLLM too

Rebased. Will leave the inheritance to future PR.

src/forge/actors/policy.py

…arams

DNXie · 2025-09-08T22:45:03Z

src/forge/actors/policy.py

I read it here that tensor_parallel_size is under EngineConfig.parallel_config.tensor_parallel_size. If so, Is this implementation correct? Should the user pass the value like this instead:

policy: engine_params: parallel_config: tensor_parallel_size = 1

@pbontrager

I comment on this below, what we have is fine since parallel_config doesn't actually exist until create_engine_config is called

apps/grpo/main.py

apps/vllm/main.py

Jack-Khuu · 2025-09-08T23:28:15Z

apps/vllm/main.py

This seems funky

Does Policy need the service configs args?

They all do, every service needs it's own config for the resources it'll get. See previous comment for how this can be made smoother

I might be missing something, but where does Policy use cfg.policy.service

Jack-Khuu · 2025-09-08T23:37:39Z

apps/vllm/llama3_8b.yaml

Out of scope for this PR, but we should think about the "service in yaml pattern" when we have some breathing room

We're gonna have a pattern of excluding this field when passings args around (since X.service is not a common Agent Arg)

Could you be more clear with the suggestions?

I personally think we should have spawn_service handle this to make it less awkward but we can do that later.

Something like

await spawn_service(Policy, **cfg.policy)

where spawn_service(actor: Actor, service_config: ServiceConfig | Mapping, **kwargs)

Agreed, no action required here

Seconding the API too

src/forge/actors/policy.py

Jack-Khuu · 2025-09-09T00:37:14Z

src/forge/actors/policy.py

Are we allowing Mapping as an input type just to work around the yaml?

Yes. Any suggestions?

No need to change anything here, but worth us thinking about down the line if we should shim this out across the repo (abstration that handles all the class constructions, actors can act on pure python) so that the actor logic is simpler

Sounds reasonable. I agree. Let's not include it in this PR for now.

src/forge/actors/policy.py

Jack-Khuu · 2025-09-09T00:45:35Z

tests/unit_tests/test_policy_config.py

nit: Can we make this a physical test artifact file instead of the tempfile if we're testing loading?

maybe unit_tests/resources/test_policy.yaml

Co-authored-by: Jack-Khuu <[email protected]>

pbontrager

Thank you! I left a few final comments for things to change but I'll approve this now so you can land afterwards.

src/forge/actors/policy.py

Jack-Khuu · 2025-09-09T16:41:03Z

apps/vllm/main.py

Looks like there's duplicate logic here

Jack-Khuu

Thanks for doing this!!

Commented on some code that got duplicated from code suggestions/rebase, but that's it

…gineconfig

DNXie and others added 9 commits August 21, 2025 16:10

Add reward interface, math reward, unit tests

da21e1d

Merge branch 'meta-pytorch:main' into main

5c72908

Merge branch 'meta-pytorch:main' into main

b4d7a61

Merge branch 'meta-pytorch:main' into main

02d77c6

Merge branch 'meta-pytorch:main' into main

fd1d38b

Merge branch 'meta-pytorch:main' into main

f79beee

Merge branch 'meta-pytorch:main' into main

d8d775a

Add explicit from_dict methods for PolicyConfig and WorkerConfig

7301e10

remove

64687d9

DNXie requested a review from pbontrager September 2, 2025 19:57

meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Sep 2, 2025

DNXie changed the title ~~Add YAML-based configuration support for vLLM main~~ [WIP] Add YAML-based configuration support for vLLM main Sep 2, 2025

pbontrager reviewed Sep 2, 2025

View reviewed changes

Jack-Khuu reviewed Sep 2, 2025

View reviewed changes

apps/vllm/main.py Outdated Show resolved Hide resolved

apps/vllm/main.py Outdated Show resolved Hide resolved

DNXie added 11 commits September 2, 2025 20:41

remove policy config

9278d75

update grpo.main

d2d7107

fixed dict attribute error, but still a buggy version

14b5e4a

update config

38f7927

for debug

2a1e021

fix the bug

412398c

clean up

d998061

lint

8d38eb8

add torchstore to dependencies

a3e755d

fix typo

9dd396b

remove a test file that causes import error

935fdc1

Jack-Khuu reviewed Sep 4, 2025

View reviewed changes

apps/vllm/main.py Outdated Show resolved Hide resolved

DNXie changed the title ~~[WIP] Add YAML-based configuration support for vLLM main~~ Add YAML-based configuration support for vLLM main Sep 4, 2025

DNXie requested a review from joecummings September 4, 2025 17:26

DNXie added 2 commits September 4, 2025 10:58

make worker config inherit engineargs

35fd71e

add unit test

187a65d

solve unit test dep

5815656

pbontrager reviewed Sep 8, 2025

View reviewed changes

DNXie and others added 10 commits September 8, 2025 14:13

revert back unit_test.yaml and remove config for grpo/main

2a31156

refactor config

4778336

rename WorkerConfig to EngineConfig and all worker_params to engine_p…

cb42997

…arams

fix test

eab380a

Merge branch 'main' into add_config_rl

d575409

rebase

a72f4de

fix lint

b19fe24

adding from_dict to samling overrides

fc809f8

minor.

6ca7c2b

fix test set

f1c24fb

DNXie commented Sep 8, 2025

View reviewed changes

fix lint and add test for nested field

4dc2e89

Jack-Khuu reviewed Sep 9, 2025

View reviewed changes

DNXie and others added 6 commits September 8, 2025 17:48

Update src/forge/actors/policy.py

00c7fc9

Co-authored-by: Jack-Khuu <[email protected]>

Update apps/vllm/main.py

4445624

Co-authored-by: Jack-Khuu <[email protected]>

Update apps/grpo/main.py

a7dfd02

Co-authored-by: Jack-Khuu <[email protected]>

rename engineConfig to EngineArgOverrides

1ed76c4

remove a redundant check

c38685f

fix lint

4191fa6

pbontrager approved these changes Sep 9, 2025

View reviewed changes

src/forge/actors/policy.py Outdated Show resolved Hide resolved

src/forge/actors/policy.py Outdated Show resolved Hide resolved

Jack-Khuu reviewed Sep 9, 2025

View reviewed changes

apps/vllm/main.py Outdated

Copy link

Contributor

Jack-Khuu Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there's duplicate logic here

Jack-Khuu approved these changes Sep 9, 2025

View reviewed changes

DNXie added 4 commits September 9, 2025 14:11

rename samplingoverrides to samplingconfig, engineargsoverrides to en…

327828b

…gineconfig

rename, remove redundant logic, refactor

fe9acae

fix lint

23e5ef6

fix CI

7b904fc

DNXie merged commit c597698 into meta-pytorch:main Sep 9, 2025
5 checks passed

DNXie deleted the add_config_rl branch September 10, 2025 19:11

Add YAML-based configuration support for vLLM main #116

Add YAML-based configuration support for vLLM main #116

Uh oh!

Conversation

DNXie commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Run vllm/main:

Test Run grpo/main:

Unit test

Test vllm_args

Uh oh!

pbontrager left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pbontrager left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DNXie Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pbontrager left a comment

DNXie commented Sep 2, 2025 •

edited

Loading

Test `vllm_args`

DNXie Sep 8, 2025 •

edited

Loading