[RL][BugFix] Fix missing tokenizer error for token-in-token-out #23904
Conversation
Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com>
Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com>
22quinn
requested review from
DarkLight1337,
aarnphm,
robertgshaw2-redhat and
simon-mo
as code owners
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a bug where using skip_tokenizer_init would cause a crash when making token-in-token-out requests. The changes correctly handle cases where the tokenizer is not initialized by making it optional in several functions. A new test is also added to cover this scenario. My review found a critical issue where a string prompt would still cause a crash if the tokenizer is not initialized. I've provided a code suggestion to add a check and raise a proper error in this case, improving the robustness of the implementation.
Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com>
Co-authored-by: Cyrus Leung <cyrus.tl.leung@gmail.com> Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com>
22quinn
added
ready
Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com>
Head branch was pushed to by a user without write access
| MODEL_NAME, | ||
| allow_patterns=["*"], | ||
| cache_dir="/tmp/qwen3_06b", | ||
| ignore_patterns=["tokenizer*", "vocab*"]) |
There was a problem hiding this comment.
also ignore the safetensors to accelerate test time.
There was a problem hiding this comment.
ignoring safetensors will cause failure, unless we set load format to be dummy.
RuntimeError: Cannot find any model weights with
/tmp/qwen3_06b/models--Qwen--Qwen3-0.6B/snapshots/c1899de289a04d12100db370d81485cdf75e47ca
| from ...utils import RemoteOpenAIServer | ||
|
|
||
| MODEL_NAME = "Qwen/Qwen3-0.6B" | ||
| MODEL_PATH = "/tmp/qwen3_06b" |
There was a problem hiding this comment.
better use a tempdir without hardcoding a path.
|
failures are unrelated, merging. |
…-project#23904) Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com> Co-authored-by: Cyrus Leung <cyrus.tl.leung@gmail.com>
…-project#23904) Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com> Co-authored-by: Cyrus Leung <cyrus.tl.leung@gmail.com>
3 participants
Purpose
Token-in-token-out is a common use case for RL. Tokenizer is not needed.
Without this PR the API server would complain:
Test Plan
pytest tests/entrypoints/openai/test_token_in_token_out.py
Test Result
Passed
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.