Configurable sampling parameter for generator

[gitlost-murali]

Generator class in src/forge/actors/generator.py now takes the number of completions (n) to be specified per request, instead of always using the default from the generator's sampling parameters.

Generator request flexibility and parameter handling

• The generate method in Generator now accepts an optional n parameter, allowing callers to specify the number of completions to generate per request. If n is not provided, the default from self.sampling_params.n is used.
• Internal logic in generate creates modified sampling parameters if n is overridden, ensuring all downstream code uses the correct value for n and related settings. This includes passing the updated parameters to tokenization, request processing, and child request creation.

Unit testing

• Added a new test test_generate_n_parameter_logic to verify that parameter overriding works as expected, including object identity checks and value assertions.

Note: This feature is useful when we extend to multi-turn rollouts where we'd generate the first turn for all K rollouts (default n=K). Later, each rollout would branch out differently - making a dedicated generate call for each turn (using generate.route(n=1))

[meta-cla]

Hi @gitlost-murali!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

[meta-cla]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

[gitlost-murali]

Hi @daniellepintz / @felipemello1 / @joecummings

Any update on this MR? Do I need to change something in the MR or what are your thoughts on this MR?

My overall goal is to integrate multi-turn rollouts. A first step into it would be enable generator have n (num_samples) argument as each trajectory can branch differently after the first generation (n = default). Later, I want to integrate an echo env which doesn't change the current training setup. Later, I can add another multi-turn env which would involve maintaining loss mask, interleaved response tokens, etc

[felipemello1]

hey @gitlost-murali , thanks for the PR! I think we can change the API a bit.

My understanding is that you want to be able to set the sampling_params for every generate call. Your current need is to change n, but someone else may want to change the temperature.

What if we do this instead:

async def generate(
        self, 
		prompt: str, 
        *, 
         priority: int = 0, 
         sampling_params: SamplingParams | None = None
    ) -> list[Completion]:
    
    params = sampling_params or self.sampling_params

you would just need to make sure that the logic in post_init still applies

[felipemello1]

regarding multiturn, i just put this PR up. Its WIP, but i am hoping to have a working recipe by early next week #567

[gitlost-murali]

Thanks! I updated the code to enable passing custom sampling_params. Note; To maintain the post_init logic, the output_kind would be set to RequestOutputKind.FINAL_ONLY

Amazing! Just read the RFC. Let me know if I can contribute somehow to multi-turn rollouts

[felipemello1]

lgtm! Will merge after CI is green

[gitlost-murali]

There was a linting issue earlier. Sorry about that
Can you re-trigger the CI-checks/auto-merge? Thanks!

[felipemello1]

no worries. You might need

pre-commit install
pre-commit run --all-files

to handle all the linter issues