FP8: Load model on-the-fly in vLLM#3717
Conversation
Summary of ChangesHello @andrewor14, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the FP8 quantization logic within the Unsloth framework to leverage new capabilities in vLLM (version 0.12.0 and above). Instead of performing an offline quantization step, models can now be quantized on-the-fly when loaded, simplifying the workflow and improving efficiency. The changes involve updating model loading functions to accept a Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors the FP8 model loading to leverage vLLM's on-the-fly quantization for versions 0.12.0 and newer. This is a solid improvement, as it eliminates the need for offline quantization and the creation of a temporary model. The implementation across llama.py, loader.py, loader_utils.py, and vision.py is well-executed, correctly checking the vLLM version to conditionally skip the offline process. My feedback consists of a minor stylistic suggestion to improve the readability of boolean checks, making them more idiomatic to Python.
| ) | ||
|
|
||
| fp8_mode = None | ||
| if load_in_fp8 != False: |
There was a problem hiding this comment.
For boolean checks, it's more idiomatic in Python to use the truthiness of the value directly rather than comparing with False. The load_in_fp8 parameter can be True, False, or a string like 'block'. Both True and non-empty strings are truthy, while False is falsy. Using if load_in_fp8: is more concise and readable, and achieves the same result.
| if load_in_fp8 != False: | |
| if load_in_fp8: |
There was a problem hiding this comment.
Yeah agree with gemini here :)
There was a problem hiding this comment.
sure I can change it, I just had it this way because I saw that's how Daniel wrote it in a few existing places
| lower_model_name = model_name.lower() | ||
|
|
||
| assert load_in_fp8 in (True, False, "block") | ||
| if load_in_fp8 != False: |
There was a problem hiding this comment.
Similar to my other comments, this check can be made more Pythonic. Instead of if load_in_fp8 != False:, you can use if load_in_fp8:. This leverages Python's truthiness evaluation and is generally considered better style for readability and conciseness.
| if load_in_fp8 != False: | |
| if load_in_fp8: |
| ) | ||
|
|
||
| fp8_mode = None | ||
| if load_in_fp8 != False: |
32cb1d7 to
f7ee75e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| load_in_4bit, | ||
| load_in_8bit, | ||
| load_in_16bit, | ||
| use_exact_model_name, | ||
| ) | ||
| model_name = _offline_quantize_to_fp8(model_name, fp8_mode) | ||
| else: |
There was a problem hiding this comment.
Enforce FP8/4bit mutual exclusion for vLLM >=0.12
When load_in_fp8 is true, get_model_name now returns the original name as soon as vLLM ≥ 0.12.0 (loader_utils.py lines 110-118), so the new_model_name is None branch here is never taken and _get_fp8_mode_and_check_settings no longer runs. With the default load_in_4bit=True, the code now proceeds to fast inference with both load_in_fp8 and use_bitsandbytes=load_in_4bit set, even though _get_fp8_mode_and_check_settings used to reject FP8 together with 4/8/16-bit loads. This yields conflicting quantization paths (fp8 on-the-fly plus bitsandbytes 4bit) and is likely to fail at runtime for users who simply enable load_in_fp8 without also disabling 4bit.
Useful? React with 👍 / 👎.
|
@andrewor14 Oh thanks - would this be backwards compatible? |
Yeah, this only affects vllm >= 0.12.0. Behavior is the same as before for older versions. Just tested on 0.12.0 and 0.11.1 |
**Summary:** Existing support for `load_in_fp8=True` performs an offline quantization when loading the initial model. This is no longer necessary as of vllm==0.12.0 (after vllm-project/vllm#23014), where we can quantize the model on-the-fly when we load it: ``` llm = LLM( ... hf_overrides={ "quantization_config_dict_str": json.dumps(torchao_config), }, ) ``` **Note:** Needs unslothai/unsloth-zoo#380 **Test Plan:** https://gist.github.com/andrewor14/5b85119fae46845d07b608d420907423
for more information, see https://pre-commit.ci
The original implementation bypasses the FP8 mapper entirely for vllm >= 0.12.0, meaning models like Llama-3.2-1B-Instruct and Qwen3-8B that have pre-quantized FP8-Block/FP8 checkpoints would never use them. This fixes the priority order: 1. Mapper has a pre-quantized model -> use it (always) 2. Mapper has no match + vllm >= 0.12.0 -> on-the-fly FP8 via torchao 3. Mapper has no match + vllm < 0.12.0 -> offline quantization Changes: - loader_utils.py: Move vllm >= 0.12.0 check after mapper lookups - loader.py: Set load_in_fp8=False when mapper resolves to a pre-quantized model to prevent double quantization Tested on B200 with Llama-3.2-1B-Instruct and Qwen3-8B. Corrected code produces results matching baseline (pre-quantized path preserved).
4451af9 to
2150655
Compare
|
Rebased on latest main and pushed a fix for the mapper bypass issue. Problem: The original implementation placed the Fix: Moved the vllm version check to after all mapper lookups, so the priority is:
Also added a guard in Testing (B200, vllm 0.15.1, 61-step SFT + LoRA): Llama-3.2-1B-Instruct:
Qwen3-8B:
The corrected version matches baseline behavior for mapped models (losses and grad norms are nearly identical). The on-the-fly path is preserved for models not in the mapper. Remaining note: |
Summary: Existing support for
load_in_fp8=Trueperforms an offline quantization when loading the initial model. This is no longer necessary as of vllm==0.12.0 (after vllm-project/vllm#23014), where we can quantize the model on-the-fly when we load it:Note: Needs unslothai/unsloth-zoo#380
Test Plan:
https://gist.github.com/andrewor14/5b85119fae46845d07b608d420907423