[V1] Support bad_words in sampler

[22quinn]

Follows the implementation in vllm/logits_process.py

There were two choices on where to tokenise bad words and the second path was taken:

1. Pass both bad_words and tokenizer via requests, but realized CachedTokenizer could not be pickled
2. Tokenize bad words by adding SamplingParams.update_from_tokenizer and invoke from engine processor.

E2E test:
Prompt: 'Hello, my name is', temperature=0, top_p=1.0
Generated text:

• No bad_words: ' J.C. and I am a student at the University of California, Berkeley'
• bad_words=["at the"]: ' J.C. and I am a student at a private school in the city'
• bad_words=["at the", "school"]: ' J.C. and I am a student at a private college in the city'

from vllm import LLM, SamplingParams
prompts = [
    "Hello, my name is",
]
sampling_params = SamplingParams(temperature=0, top_p=1.0)
sampling_params = SamplingParams(temperature=0, top_p=1.0, bad_words=["at the"])
sampling_params = SamplingParams(temperature=0, top_p=1.0, bad_words=["at the", "school"])
llm = LLM(model="facebook/opt-125m", enforce_eager=True)
outputs = llm.generate(prompts, sampling_params)

for i, output in enumerate(outputs):
    prompt = output.prompt
    generated_text = output.outputs[0].text
    print(f"Prompt: {prompt!r}")
    print(f"Generated text: {generated_text!r}")

part of #13058

[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.

🚀

[22quinn]

@WoosukKwon Could someone help to review this? Many thanks!

[njhill]

@22quinn I'm not too keen on the way that this is currently implemented (I know you just ported the v0 impl but my thoughts apply to that too).

IMHO instead of rushing this into V1 we could temporarily include bad_words as one of the initially unsupported parameters, and add an alternative implementation along the lines of what is described in vllm slack here.

A first step to supporting this would be to add a flag to the existing allowed_token_ids impl to control whether the provided token ids are allow or disallow. So we can add a disallowed_token_ids sampling parameter and you're not allowed to set both allowed and disallowed at the same time. I think this should be a pretty small change.

Once we have that, a much better bad_words impl should be fairly simple to add in an outer layer. The main downside is that part of this will need separate logic for the async and sync cases.

Edit: I just remembered that in this case we only want the disallowed token(s) to apply to the first generated token. We could still achieve this in the outer layer without a specific param, but it would involve a bit more overhead (send a req with max_tokens=1 and disallowed_tokens, then continue without disallowed_tokens).

cc @houseroad @AlpinDale

[AlpinDale]

As previously discussed offline, I believe bad_words is an incomplete solution to a very real problem; banning only the last token in the provided sequence is not useful at all, ideally we want a method to ban the string in its entirety. This is unfortunately impossible with only the sampler layer and a logits processor. We'd need scheduler changes to enable rollbacks and buffering of tokens. Perhaps using multi-step scheduling code-path for the buffering.

[njhill]

As previously discussed offline, I believe bad_words is an incomplete solution to a very real problem; banning only the last token in the provided sequence is not useful at all, ideally we want a method to ban the string in its entirety.

@AlpinDale agree, with the flaw of not doing string-based matching so it could still be possible for the banned string to occur.

We'd need scheduler changes to enable rollbacks and buffering of tokens. Perhaps using multi-step scheduling code-path for the buffering.

Did you read my proposal? I think there is a fairly simple and less invasive way to do it.

[22quinn]

with the flaw of not doing string-based matching so it could still be possible for the banned string to occur

@njhill so it's possible for different token sequences to be decoded to the same word sequence? Otherwise I think current v0 implementation still fulfills its purpose although it rolls back the last token only.

[mergify]

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @22quinn.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

[njhill]

with the flaw of not doing string-based matching so it could still be possible for the banned string to occur

@njhill so it's possible for different token sequences to be decoded to the same word sequence?

Yes, v0 already has this limitation.

[njhill]

Thanks @22quinn for the good work here. The impl is quite inefficient/flawed as discussed (not your fault, I know this is just v0 ported), but we would like to have the functionality in v1 asap so it would be great to get this in and we can optimize it as a follow-on.

[22quinn]

@njhill Thanks for the review and clarification on the current limitations! I've addressed the comments and tested with unit tests and E2E runs.

[njhill]

Thanks @22quinn, looks great. There is one bug which needs addressing.

[njhill]

LGTM, thanks for jumping on this so fast @22quinn

[22quinn]

LGTM, thanks for jumping on this so fast

No problem! Excited to become a contributor and look forward to learning and contributing more!

[simon-mo]

Test failed https://buildkite.com/vllm/ci/builds/14971/canvas?sid=01957347-78b6-407a-921f-c7a82847a7ed#01957347-7a52-4f3d-972b-78decfdc6577/206-12701

[DarkLight1337]

Needs rebase