-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make initialization of tokenizer and detokenizer optional #3748
Merged
Merged
Changes from all commits
Commits
Show all changes
47 commits
Select commit
Hold shift + click to select a range
0ca0b3d
[optional-tokenizer] make tokenzier optional in initialization
EricDingNVD a50f0c7
[tokenizer] make detokenization optional
EricDingNVD e144c2e
[tokenizer] fix parameter description
EricDingNVD 5fb16f2
[tokenizer] fix initialize engine args
EricDingNVD 904edcc
[tokenizer] fix format
EricDingNVD cfc2660
[tokenization] fix arg parser field
EricDingNVD 013a36a
[tokenizer] fix the order of initializing tokenizer and de-tokenizer
EricDingNVD fb3eefd
[tokenizer] Never disable tok in LLM initialization
EricDingNVD 07cc2e5
[tokenizer] Add flag value to log info to help debug
EricDingNVD 5b30825
[tokenizer] fix type
EricDingNVD 676256f
[tokenizer] fix yapf errors
EricDingNVD f7cd883
[tokenizer] fix formatting
EricDingNVD 8dfb59b
Merge branch 'vllm-project:main' into optional-tokenizer
GeauxEric 0ea8446
[optional-tokenizer] make tokenzier optional in initialization
EricDingNVD a7be734
[tokenizer] make detokenization optional
EricDingNVD 1e613f6
[tokenizer] fix parameter description
EricDingNVD 3b94adb
[tokenizer] fix initialize engine args
EricDingNVD eab1dd7
[tokenizer] fix format
EricDingNVD 58ccf64
[tokenization] fix arg parser field
EricDingNVD a0d1405
[tokenizer] fix the order of initializing tokenizer and de-tokenizer
EricDingNVD 0af6b47
[tokenizer] Never disable tok in LLM initialization
EricDingNVD a497ed9
[tokenizer] Add flag value to log info to help debug
EricDingNVD af078a8
[tokenizer] fix type
EricDingNVD ad2c920
[tokenizer] fix yapf errors
EricDingNVD 78d4091
[tokenizer] fix formatting
EricDingNVD 4f67490
[tokenizer] fix EngineArgs
EricDingNVD a093589
Merge branch 'optional-tokenizer' of github.com:GeauxEric/vllm into o…
EricDingNVD 59fc5eb
[tokenizer] fix init LLM
EricDingNVD ac7a3d4
[tokenizer] rename the flag
EricDingNVD 400224d
[tokenizer] rename the flag
EricDingNVD 6941628
[tokenizer] add integration test
EricDingNVD 7e851ac
Merge branch 'optional-tokenizer' of github.com:GeauxEric/vllm into o…
EricDingNVD ad4da7c
[tokenizer] add integration test
EricDingNVD 3f60c11
Merge branch 'optional-tokenizer' of github.com:GeauxEric/vllm into o…
EricDingNVD 4b3c5e3
[tokenizer] test generate based on prompt token ids
EricDingNVD 87c695b
[tokenizer] more tests
EricDingNVD aa5ec54
[tokenizer] consider finialize sequence
EricDingNVD 4943cbd
Merge branch 'main' of github.com:GeauxEric/vllm into optional-tokenizer
EricDingNVD c208c77
Merge branch 'main' into optional-tokenizer
EricDingNVD 68f77b1
Merge branch 'main' into optional-tokenizer
EricDingNVD c0951f3
[tokenizer] fix integration test
EricDingNVD 5f8b5fd
Merge branch 'main' of github.com:GeauxEric/vllm into optional-tokenizer
EricDingNVD 47dce6e
[tokenizer] merge with main
EricDingNVD 50a7fad
[tokenizer] log warning if eos_token_id is None
EricDingNVD 7c25549
Merge branch 'main' of github.com:GeauxEric/vllm into optional-tokenizer
EricDingNVD 5f2b8ed
[tokenizer] work around mypy errors
EricDingNVD e584673
Merge remote-tracking branch 'upstream/main' into optional-tokenizer
ywang96 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
import pytest | ||
|
||
from vllm.entrypoints.llm import LLM | ||
from vllm.sampling_params import SamplingParams | ||
|
||
|
||
@pytest.mark.parametrize("model", ["facebook/opt-125m"]) | ||
def test_skip_tokenizer_initialization(model: str): | ||
# This test checks if the flag skip_tokenizer_init skips the initialization | ||
# of tokenizer and detokenizer. The generated output is expected to contain | ||
# token ids. | ||
llm = LLM(model=model, skip_tokenizer_init=True) | ||
sampling_params = SamplingParams(prompt_logprobs=True, detokenize=True) | ||
with pytest.raises(ValueError) as err: | ||
llm.generate("abc", sampling_params) | ||
assert "prompts must be None if" in str(err.value) | ||
outputs = llm.generate(prompt_token_ids=[[1, 2, 3]], | ||
sampling_params=sampling_params) | ||
assert len(outputs) > 0 | ||
completions = outputs[0].outputs | ||
assert len(completions) > 0 | ||
assert completions[0].text == "" | ||
assert completions[0].token_ids |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO we should add a warning here - WDYT?
This will also affect components that use
eos_token_id
. For example,vllm/vllm/engine/llm_engine.py
Lines 344 to 346 in 0ce0539
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think it's worth having an warning here about
eos_token_id
beingNone
.