Add layer-wise KV-cache AutoQuant with forward KL - #2211
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Assisted-by: OpenAI Codex Signed-off-by: weimingc <17592131+meenchen@users.noreply.github.com>
Signed-off-by: weimingc <17592131+meenchen@users.noreply.github.com>
Signed-off-by: weimingc <17592131+meenchen@users.noreply.github.com>
Signed-off-by: weimingc <17592131+meenchen@users.noreply.github.com>
c55d772 to
d26d912
Compare
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2211 +/- ##
==========================================
+ Coverage 78.94% 79.02% +0.07%
==========================================
Files 522 523 +1
Lines 60550 60953 +403
==========================================
+ Hits 47803 48168 +365
- Misses 12747 12785 +38
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: weimingc <17592131+meenchen@users.noreply.github.com>
Signed-off-by: weimingc <17592131+meenchen@users.noreply.github.com>
Signed-off-by: weimingc <17592131+meenchen@users.noreply.github.com>
Signed-off-by: weimingc <17592131+meenchen@users.noreply.github.com>
cjluo-nv
left a comment
There was a problem hiding this comment.
Bot review (gpt-5.6-sol) — DM the bot to share feedback.
The feature solves a real gap: choosing one calibrated K/V format per attention layer under a KV-storage budget and exporting that heterogeneous mapping. However, the architectural choice is not fully justified. The repo already has AutoQuantizeKLDivSearcher for isolated forward-KL scoring, QuantRecipeHparam for swapping per-choice quantizers/grouped decisions, and BaseSearcher for checkpoint/signature lifecycle, with the existing PuLP-backed LPS already used here. The PR body explains why the current weight cost model cannot be used unchanged, but not why those abstractions cannot be generalized or subclassed with K/V quantizer attributes and a pluggable width-based cost model instead of adding a parallel 673-line search/checkpoint engine. Please document that tradeoff before approval.
I also found a storage-accounting bug for the supported asymmetric FP8-K/NVFP4-V candidate: its effective bits are averaged equally between K and V, while layers are weighted by the sum of potentially unequal K/V widths. This can violate kv_effective_bits; the existing width test even demonstrates unequal K/V projections, but no asymmetric test covers the case. The shipped recipe also imports cost_excluded_layers, while the KV recipe-to-API path silently drops that field, including *mtp*.
Test coverage is otherwise substantial, and the new source headers match LICENSE_HEADER. At +2,179/-61 across 20 files, this is also a high-risk review size; splitting the search/API work from export metadata where practical would make validation easier.
| ) | ||
|
|
||
| assert config.effective_bits is not None | ||
| actual_effective_bits = (k_bits + v_bits) / 2.0 |
There was a problem hiding this comment.
Bot comment.
This cost is exact only when K and V have equal widths. _kv_scalar_weight() explicitly supports unequal k_proj.out_features/v_proj.out_features (and the unit test uses 24 vs 16), but an FP8-K/NVFP4-V candidate is charged (k_width + v_width) * (8 + 4.5) / 2 instead of k_width * 8 + v_width * 4.5. That can select a recipe whose actual packed storage violates kv_effective_bits. Please either reject asymmetric candidates on unequal-width layers or build per-layer/per-candidate costs from the separate K and V widths, and add an asymmetric unequal-width solver test.
| disabled_layers: | ||
| - $import: base_disabled_layers | ||
|
|
||
| cost_excluded_layers: |
There was a problem hiding this comment.
Bot comment.
This field is silently ignored for KV searches: _mtq_inputs_from_auto_quantize_config() returns early in the KV branch and never forwards cost_excluded_layers. In particular, *mtp* is present in this imported unit but not in base_disabled_layers, so an MTP attention layer can remain eligible and count toward the KV budget despite this recipe appearing to exclude it. Please remove the dead field and explicitly disable the intended layers, reject cost_excluded_layers in KV recipe validation, or define and forward its KV-search semantics.
What does this PR do?
Type of change: new feature.
Adds layer-wise KV-cache AutoQuantize with isolated forward-KL sensitivity:
mtq.auto_quantize_kv_cache, using BF16/no-KV-quant logits as the reference and quantizing one eligible attention layer at a time;constraints.kv_effective_bits, with caller-declared packed cost per K/V scalar;examples/hf_ptq;The existing weight-only AutoQuant search groups decisions and derives cost from weights, so it cannot safely represent one joint K/V decision per attention layer or heterogeneous KV export. This adds a separate KV-specific search path and versioned export metadata instead of overloading the weight cost model.
Relationship to vLLM runtime support
This PR is the checkpoint producer: it searches the recipe and writes schema-v1
kv_cache_quantized_layersmetadata. The companion vllm-project/vllm#52813 is the checkpoint consumer: it reads that mapping and dispatches each attention layer.Together, the two PRs support layer-wise mixtures of full FP8 K/V and full NVFP4 K/V without any new kernel code; vLLM #52813 uses the existing uniform FP8 and NVFP4 implementations for each selected layer.
FP8-K/NVFP4-V within one layer is a separate capability. This PR can search and export that format, but vLLM #52813 deliberately rejects it because it requires the independent mixed-K/V attention-kernel implementation. Neither PR bundles that kernel work.
Usage
The shipped three-format recipe can also be run with:
Testing
main: 319 passed, 4 environment-gated failuresPrefixChange/scope_prefix) newer than the locally availabletransformers==5.4.0;Before your PR is "Ready for review"
CONTRIBUTING.md: N/AAdditional Information
disabled_layersare preserved in their existing KV format and excluded from the searched-layer bit budget.