-
Notifications
You must be signed in to change notification settings - Fork 7
backend fallback draft #50
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: alec-flowers <[email protected]>
| - To enable KV Cache Routing we need to have tokens in frontend in order to calculate hashes to match against the indexers hashes created from KV events emitted from the backend. | ||
|
|
||
| REQ 3: PostProcessing SHOULD move to the backend engine | ||
| - Post-processing is currently done on the engine side, however the intention was to co-located it with the backend where there are many CPU's sitting idle alongside the GPU's. |
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.
Is this meant to be "currently done on the frontend side"?
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.
Woops, yes.
| """Combine chat template + tokenization in one call.""" | ||
| pass | ||
|
|
||
| class VllmTokenizer(TokenizerProtocol): |
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.
Would it conceptually allow the sglang tokenizer as well?
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.
Yes, I was just providing this as a first example.
|
|
||
| **Benefits**: Offloads CPU work to backend servers, scales with engine instances | ||
|
|
||
| #### Mode 2: vLLM Native Processing (`--vllm-processing`) |
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.
Will this conceptually work with SGL and vLLM paths as well?
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.
Yes
|
|
||
| **Benefits**: Offloads CPU work to backend servers, scales with engine instances | ||
|
|
||
| #### Mode 2: vLLM Native Processing (`--vllm-processing`) |
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.
Can we think of this in terms of a "hello world backend" example, CLI args, and all - and see if it generalizes to vllm, sglang, trtllm, etc. that each implement the "interface" we defined out of the box for users of our officially supported backends?
For example, maybe any backend would accept args like --dyn-backend-preprocessor and --dyn-backend-postprocessor (subject to change, just following --dyn- prefix convention for dynamo specific args rather than framework args we passthrough), etc. and for vllm backend that happens to map to using vllm's tokenizer?
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.
Yea I wasn't sure the best UX so just tried to write something down. Open to what you are suggestion as well.
|
|
||
| # Motivation | ||
|
|
||
| Dynamo’s Rust-based frontend is providing excellent performance, but has recently been a source of friction for users. This is due to it implementing the pre- and post-processing pipelines for requests (tokenization, chat template application, reasoning parsers, etc), and if a particular piece of functionality is missing for some model, it requires work to add (even if the functionality is available in the underlying engine). |
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.
Rather calling out 'excellent', do we have a number ? , nitpick :) How does this rephrase sound ? "This is because it implements the pre- and post-processing pipelines for requests (such as tokenization, chat template application, and reasoning parsers). If a particular piece of functionality is missing for a given model, additional work is required to add it, even if the functionality already exists in the underlying engine.
| @@ -0,0 +1,222 @@ | |||
| # Native Framework Processing Fallback | |||
|
|
|||
| **Status**: Draft | |||
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.
Suggestion : Let's version it. example V.Alpha
|
|
||
| ## Goals | ||
|
|
||
| Allow the user to select between dynamo processing or native engine processing with a command line flag. The default will be the fast path of dynamo based processing with the option to add a flag: |
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.
By doing this what is the impact for the user? , to deliver this <> ,
|
|
||
| impl TokenizerAdapter for PythonTokenizerAdapter { | ||
| fn tokenize(&self, messages: &[Message], model: &str, tools: Option<&[Tool]>) | ||
| -> anyhow::Result<Vec<u32>> |
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.
"anyhow::Result<Vec>" <--This is fine for flexible error propagation. Is here is that the intent ? or pub type Result = std::result::Result<T, TokenizerError>; is more appropriate ?
|
|
||
| REQ 1: User SHOULD be able to select between dynamo processing and the backend enginge processing. | ||
|
|
||
| REQ 2: Preprocessing MUST happen on the frontend. |
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.
What does 'frontend' refer to?
| """Combine chat template + tokenization in one call.""" | ||
| pass | ||
|
|
||
| class VllmTokenizer(TokenizerProtocol): |
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.
Where this implementation lives, you will need to import vLLM and all its tokenization utilities, correct?
|
|
||
| ```rust | ||
| // Rust passes tokenizer path to Python factory | ||
| let tokenizer_path = card.get_tokenizer_path()?; |
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.
By passing only the path here, we assume the factory can figure out the type of tokenizer from that path?
| // Rust passes tokenizer path to Python factory | ||
| let tokenizer_path = card.get_tokenizer_path()?; | ||
| let adapter = Python::with_gil(|py| { | ||
| let py_tokenizer = factory.call1(py, (tokenizer_path,))?; |
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.
Should there be a protocol for the factory too?
No description provided.