[core] add extra_args to SamplingParams

[akeshet]

This PR adds an extra_args field to SamplingParams, for use by custom (out-of-tree) sampling implementations.

[github-actions]

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

🚀

[NickLucche]

Looks like pre-commit needs to be run here

[akeshet]

Looks like pre-commit needs to be run here

Fixed manually, thanks!

[NickLucche]

shouldn't this be optional? Also wouldn't it be better to have a kwargs style parameter?

[akeshet]

Since this is of type Any which includes NoneType, and has a default value of None defined, it's already effectively optional.

[njhill]

Thanks for this @akeshet, I think we definitely want to add such a parameter.

However, similar to what @NickLucche said, could we make the type Dict[str, str] initially?

We could later consider kwargs style i.e. Dict[str, Any] but I think it would be better to do that only if/when a strong need is identified.

Some rationale: we have to serialize the sampling params and so don't want to allow arbitrary types, and also we'll likely want to expose this via the OpenAI API as custom extra args.

[akeshet]

Thanks for this @akeshet, I think we definitely want to add such a parameter.

However, similar to what @NickLucche said, could we make the type Dict[str, str] initially?

We could later consider kwargs style i.e. Dict[str, Any] but I think it would be better to do that only if/when a strong need is identified.

Some rationale: we have to serialize the sampling params and so don't want to allow arbitrary types, and also we'll likely want to expose this via the OpenAI API as custom extra args.

In my particular use case, Dict[str, str] would not be sufficient, one of the extra args we need for our in house sampler is a (serializable) object.

I considered Dict[str, Any]. That doesn't actually provide any additional serializability guarantees though relative to Any, and Any really gives maximum flexibility.

If you prefer Dict[str, Any] though I can update to use that (or really, Optional[Dict[str, Any]])

[NickLucche]

If there's really no way around having Any as type here, I'd prefer Optional[Dict[str, Any]] but that is nit on my side.

[akeshet]

If there's really no way around having Any as type here, I'd prefer Optional[Dict[str, Any]] but that is nit on my side.

Done

[njhill]

In my particular use case, Dict[str, str] would not be sufficient, one of the extra args we need for our in house sampler is a (serializable) object.

@akeshet would you mind me asking what kind of object you're passing? The idea is that these should just be configuration parameters for the sampler.

[akeshet]

In my particular use case, Dict[str, str] would not be sufficient, one of the extra args we need for our in house sampler is a (serializable) object.

@akeshet would you mind me asking what kind of object you're passing? The idea is that these should just be configuration parameters for the sampler.

Yes, these are configuration parameters for our out-of-tree sampler, it's just that these parameters are not of the form Dict[str, str]

[akeshet]

@njhill I think all comments are addressed, could you take another look when convenient?

[njhill]

Thanks @akeshet, sorry for the delay. Wanted to be sure about this since it will be hard to change/remove in future once it's there.

[DarkLight1337]

Can you merge from main to try to fix the CI failures?

[akeshet]

Can you merge from main to try to fix the CI failures?

Done, just pushed a merge.

[Selkh]

"extra_args" is not avaible for api_server. Would you kind to fullfish it in "entrypoints"?

[DarkLight1337]

Do you mean entrypoints/api_server.py? This is just a demo API server and. as mentioned by the docstring at the top of the file, we will not push any new features to it. Please either use entrypoints/openai/api_server.py or create your own API server based on it.

[Selkh]

Do you mean entrypoints/api_server.py? This is just a demo API server and. as mentioned by the docstring at the top of the file, we will not push any new features to it. Please either use entrypoints/openai/api_server.py or create your own API server based on it.

No, actually I just mean entrypoints/openai/api_server.py. It seems "extra_args" will be lost during creaing sampling_params by "SamplingParams.from_optional(...)" in entrypoints/openai/protocol.py (https://github.com/vllm-project/vllm/blob/main/vllm/entrypoints/openai/protocol.py#L480)

[DarkLight1337]

I see, @akeshet @njhill is it reasonable to add this as well?

[njhill]

@DarkLight1337 yes I think this is the natural next step here. To route unrecognized fields in CompletionRequest / ChatCompletionRequest into this dict. I'm not sure what pydantic related changes would be needed for that. PRs welcome!

[njhill]

Opened a new issue for the follow-on task to expose in / populate from the OpenAI endpoints: #16802