[Misc][Quantization] Remove kvcache quant weight related patches. - #11053
Conversation
|
👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:
If CI fails, you can run linting and testing checks locally according Contributing and Testing. |
Summary of ChangesHello, 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 cleans up the codebase by removing temporary patches related to KV cache quantization weight loading. The logic for remapping checkpoint weight names has been migrated to a centralized WeightsMapper approach within modelslim_config.py, improving maintainability and reducing the need for intrusive monkey-patching. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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 the 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 counterproductive. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
Suggested PR Title:
[Ops][Misc] Refactor weight loading and remove obsolete patchesSuggested PR Summary:
### What this PR does / why we need it?
This PR removes obsolete weight utility and GQA C8 patches (`patch_weight_utils.py` and `patch_gqa_c8.py`) and refactors the quantization configuration in `modelslim_config.py`. Specifically, it replaces `get_cache_scale` with `get_cache_scale_mapper` to return a `WeightsMapper` for C8, FA, and indexer quantization parameters.
Feedback on this PR includes:
- Renumbering the patch index sections in `vllm_ascend/patch/__init__.py` to maintain a continuous sequence after the removal of sections 21 and 26.
- Using the pre-computed `self.enable_c8_quant` attribute in `modelslim_config.py` instead of directly querying `self.quant_description` to avoid redundant dictionary lookups.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Not specified in the PR.| # Future Plan: | ||
| # The maybe_remap_kv_scale_name function of the community is reconstructed to support | ||
| # multiple backends. | ||
| # ** 22. File: worker/patch_v2/patch_input_batch.py** |
There was a problem hiding this comment.
Since section 21 (worker/patch_weight_utils.py) has been removed, the section numbering is now discontinuous (skipping 21). Please renumber this section to 21 and shift the subsequent sections (22, 23, 24, etc.) accordingly to maintain a consistent and continuous index of patches.
| # ** 22. File: worker/patch_v2/patch_input_batch.py** | |
| # ** 21. File: worker/patch_v2/patch_input_batch.py** |
| # or when the GQA model's weight names are aligned with the parameter | ||
| # names expected by the quantisation backend. | ||
| # ** 27. File: worker/patch_qwen3vl.py** | ||
| # ** 26. File: worker/patch_qwen3vl.py** |
| # Future Plan: | ||
| # Remove this patch when vllm-ascend supports pattern matching for this fused kernel. | ||
| # ** 28. File: worker/patch_qwen3_dflash.py** | ||
| # ** 27. File: worker/patch_qwen3_dflash.py** |
| # can be extended by hardware plugins like vllm-ascend. | ||
| # | ||
| # ** 29. File: platform/patch_mamba_manager.py** | ||
| # ** 28. File: platform/patch_mamba_manager.py** |
| if self.quant_description.get("kv_cache_type") != "C8": | ||
| def get_cache_scale_mapper(self) -> "WeightsMapper | None": | ||
| suffix_map = {} | ||
| if self.quant_description.get("kv_cache_type") == "C8": |
There was a problem hiding this comment.
Use the pre-computed boolean attribute self.enable_c8_quant instead of querying self.quant_description directly. This is more consistent with how self.enable_fa_quant and self.enable_indexer_quant are used below, and avoids redundant dictionary lookups.
| if self.quant_description.get("kv_cache_type") == "C8": | |
| if self.enable_c8_quant: |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
| # we have enhanced the maybe_remap_kv_scale_name function. | ||
| # Future Plan: | ||
| # The maybe_remap_kv_scale_name function of the community is reconstructed to support | ||
| # multiple backends. | ||
| # ** 22. File: worker/patch_v2/patch_input_batch.py** |
There was a problem hiding this comment.
Need to check the serial number after the patch is removed.
There was a problem hiding this comment.
In addition to this, LGTM.
There was a problem hiding this comment.
Update the serial number
a8aff3e to
239e48c
Compare
|
/rerun Rerun:
|
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Signed-off-by: menogrey <1299267905@qq.com>
Signed-off-by: menogrey <1299267905@qq.com>
Signed-off-by: menogrey <1299267905@qq.com>
dbca1aa to
6b524aa
Compare
Signed-off-by: menogrey <1299267905@qq.com>
Signed-off-by: menogrey <1299267905@qq.com>
|
/weekly multi-node-glm4.7-w8a8c8-layerwise
|
### What this PR does / why we need it? #11053 introduce a UT error. This PR fix it. - vLLM version: v0.23.0 - vLLM main: vllm-project/vllm@1f486d9 Signed-off-by: menogrey <1299267905@qq.com>
…lm-project#11053) ### What this PR does / why we need it? After vllm-project/vllm#43167 , we can remove our two patches about kvcache quant weight. Should be merged after our main2main at vllm-tag 0.23.0 and vllm-main-0605. ### Does this PR introduce _any_ user-facing change? ### How was this patch tested? local tested model: Qwen3-32B-W8A8C8 DeepSeek-V3.1-W4A8C8 - vLLM version: v0.23.0 - vLLM main: vllm-project/vllm@1f486d9 --------- Signed-off-by: menogrey <1299267905@qq.com>
### What this PR does / why we need it? vllm-project#11053 introduce a UT error. This PR fix it. - vLLM version: v0.23.0 - vLLM main: vllm-project/vllm@1f486d9 Signed-off-by: menogrey <1299267905@qq.com>
Signed-off-by: menogrey <1299267905@qq.com>
…hes." (#11615) Reverts #11053 - vLLM version: v0.23.0 - vLLM main: vllm-project/vllm@1f486d9 Signed-off-by: menogrey <1299267905@qq.com>
…lm-project#11053) ### What this PR does / why we need it? After vllm-project/vllm#43167 , we can remove our two patches about kvcache quant weight. Should be merged after our main2main at vllm-tag 0.23.0 and vllm-main-0605. ### Does this PR introduce _any_ user-facing change? ### How was this patch tested? local tested model: Qwen3-32B-W8A8C8 DeepSeek-V3.1-W4A8C8 - vLLM version: v0.23.0 - vLLM main: vllm-project/vllm@1f486d9 --------- Signed-off-by: menogrey <1299267905@qq.com>
…lm-project#11053) ### What this PR does / why we need it? After vllm-project/vllm#43167 , we can remove our two patches about kvcache quant weight. Should be merged after our main2main at vllm-tag 0.23.0 and vllm-main-0605. ### Does this PR introduce _any_ user-facing change? ### How was this patch tested? local tested model: Qwen3-32B-W8A8C8 DeepSeek-V3.1-W4A8C8 - vLLM version: v0.23.0 - vLLM main: vllm-project/vllm@1f486d9 --------- Signed-off-by: menogrey <1299267905@qq.com> Signed-off-by: Spicy-Stick <873805887@qq.com>
### What this PR does / why we need it? vllm-project#11053 introduce a UT error. This PR fix it. - vLLM version: v0.23.0 - vLLM main: vllm-project/vllm@1f486d9 Signed-off-by: menogrey <1299267905@qq.com> Signed-off-by: Spicy-Stick <873805887@qq.com>
…hes." (#11615) Reverts vllm-project/vllm-ascend#11053 - vLLM version: v0.23.0 - vLLM main: vllm-project/vllm@1f486d9 Signed-off-by: menogrey <1299267905@qq.com> Signed-off-by: xqchen7 <chenxueqing7@huawei.com>
…lm-project#11053) ### What this PR does / why we need it? After vllm-project/vllm#43167 , we can remove our two patches about kvcache quant weight. Should be merged after our main2main at vllm-tag 0.23.0 and vllm-main-0605. ### Does this PR introduce _any_ user-facing change? ### How was this patch tested? local tested model: Qwen3-32B-W8A8C8 DeepSeek-V3.1-W4A8C8 - vLLM version: v0.23.0 - vLLM main: vllm-project/vllm@1f486d9 --------- Signed-off-by: menogrey <1299267905@qq.com>
### What this PR does / why we need it? vllm-project#11053 introduce a UT error. This PR fix it. - vLLM version: v0.23.0 - vLLM main: vllm-project/vllm@1f486d9 Signed-off-by: menogrey <1299267905@qq.com>
…hes." (vllm-project#11615) Reverts vllm-project#11053 - vLLM version: v0.23.0 - vLLM main: vllm-project/vllm@1f486d9 Signed-off-by: menogrey <1299267905@qq.com>
What this PR does / why we need it?
After vllm-project/vllm#43167 , we can remove our two patches about kvcache quant weight.
Should be merged after our main2main at vllm-tag 0.23.0 and vllm-main-0605.
Does this PR introduce any user-facing change?
How was this patch tested?
local tested model:
Qwen3-32B-W8A8C8
DeepSeek-V3.1-W4A8C8