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 Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
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% 78.70% -0.25%
==========================================
Files 522 524 +2
Lines 60550 62255 +1705
==========================================
+ Hits 47803 48996 +1193
- Misses 12747 13259 +512
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.
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.
Re-review: the feature addresses a real gap—selecting one calibrated K/V format per attention layer under a KV-storage budget and exporting the heterogeneous mapping—but all three prior blocking concerns remain.
- Design (approval-blocking): the updated PR body explains why the existing weight cost model cannot be used unchanged, but it still does not explain why the existing
AutoQuantizeKLDivSearcherisolated forward-KL flow,QuantRecipeHparamchoice swapping/grouping, andBaseSearchercheckpoint lifecycle cannot be generalized for K/V quantizer attributes and a pluggable width-based cost model. The already-imported PuLP alternative is exposed throughLPSand is appropriately reused. Before approval, please document why extending/subclassing those in-repo abstractions is not viable instead of adding a parallel 684-line search/checkpoint implementation. - Critical correctness: asymmetric FP8-K/NVFP4-V accounting still averages K/V bits and then multiplies by total width, so unequal K/V widths can violate the advertised
kv_effective_bitsconstraint. Removing this candidate from the shipped recipe limits exposure but does not fix the public API, which still accepts and advertises it; the new test still uses equal K/V widths. - Critical recipe/API mismatch: the shipped recipe still imports
base_cost_excluded_layers, but the KV recipe conversion returns before forwarding that field.*mtp*is not inbase_disabled_layers, so the recipe can search and budget MTP attention despite appearing to exclude it.
The new source/test headers match LICENSE_HEADER, and test coverage is otherwise substantial. At 22 files and +2,357/-67, the PR remains difficult to validate as one unit; splitting search/API from export metadata would reduce risk.
Signed-off-by: weimingc <17592131+meenchen@users.noreply.github.com>
|
Addressed the latest review in commit
Validation: 38 focused changed-area tests passed; all changed-file pre-commit hooks passed, including recipe validation. |
cjluo-nv
left a comment
There was a problem hiding this comment.
Bot review (gpt-5.6-sol) — DM the bot to share feedback.
Re-review/design review: this solves a real gap—choosing one calibrated K/V format per attention boundary under a width-weighted cache budget and exporting the heterogeneous mapping. I checked the existing alternatives: AutoQuantizeKLDivSearcher/_AutoQuantizeBaseSearcher already provide isolated forward-KL search flow, QuantRecipeHparam provides candidate quantizer swapping/grouping, and BaseSearcher provides checkpoint lifecycle; the already-imported PuLP path is exposed through LPS and is reused here. Pydantic/OmegaConf cover schema/config composition but not the model-side search itself.
💬 Author replied in commit 34a249597 that the existing abstractions are tied to linear-module grouping, parameter-numel cost, and weight-search checkpoint contracts, and that generalizing them risks changing existing AutoQuant behavior. The PR body now records that tradeoff, so the deterministic design gate is substantially addressed; however, a parallel 714-line search/checkpoint implementation beside those existing abstractions still warrants owner/architect sign-off rather than automated approval.
💬 Addressed in 34a249597: asymmetric FP8-K/NVFP4-V now fails before mutation when any eligible layer has unequal K/V widths, with a focused unequal-width regression. This resolves the prior storage-accounting correctness issue.
💬 Addressed in 34a249597: the KV recipe removes unsupported cost_excluded_layers, moves *mtp* into disabled_layers, validation rejects the unsupported field, and recipe-to-public-API coverage verifies forwarding. This resolves the prior recipe/API mismatch.
Test coverage is substantial and the new-file headers match LICENSE_HEADER. The remaining concern is reviewability and architectural ownership: at 22 files and +2,426/-71, splitting the search/API work from export metadata would materially reduce risk if practical.
Signed-off-by: weimingc <17592131+meenchen@users.noreply.github.com>
|
Addressed the latest findings in signed+DCO commit
Evidence:
|
Signed-off-by: weimingc <17592131+meenchen@users.noreply.github.com>
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 HF recipe path now also composes GEMM and KV optimization as ordered stages on the same in-memory model:
The selected GEMM weight/activation QDQ remains enabled during KV scoring while its calibration state is frozen, so KV sensitivity is measured against the model that will be exported. Weight and KV searches retain independent constraints, methods, candidates, reports, and resumable checkpoint paths.
The existing weight-only
AutoQuantizeKLDivSearcherandQuantRecipeHparamoperate on linear-module weight/input/output quantizer recipes. Their grouping follows runtime-fusion rules and their cost is derived from parameternumel. KV search instead owns one joint K/V choice at an attention boundary, costs resident K/V scalar widths, and checkpoints calibrated K/V scale tensors together with per-layer search progress. Generalizing the existing hparam would therefore change its grouping, cost, and checkpoint contracts and risk existing weight AutoQuant behavior; subclassingBaseSearcherwould not remove those KV-specific responsibilities. The KV path remains separate internally while reusing the commonLPSsolver, calibration/quantizer utilities, andsafe_load/safe_saveprimitives. YAML composition sequences the independent stages without merging their solver or checkpoint contracts.Model-family support status
Support remains architecture-driven: there are no model-name checks, checkpoint paths, fixed layer lists, or campaign flags.
config.architectures = None, as observed with this family.Additional fail-closed behavior:
disabled_layerspreserve fixed or unsupported layers;fp8_k_nvfp4_v; FP8 and NVFP4 preset names are unchanged;cost_excluded_layers; non-KV-cache modules such as MTP usedisabled_layers, which removes them from scoring and budget accounting;Composition status and open review gates
The producer can export a uniform FP8-weight or mixed-weight checkpoint together with schema-v1
kv_cache_quantized_layersmetadata. Two composed Qwen3-8B checkpoints were generated from the published branch and their producer-side checkpoint inventories and mappings validated.The following review gates remain open at head
452328d:ModelOptMixedPrecisionConfig; uniform FP8/NVFP4 weight dispatch must also consume the map before composed uniform-weight checkpoints are deployment-ready.Until these gates are addressed, the composed recipes are producer-validation functionality and should not be treated as end-to-end deployment-ready with the linked consumer.
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.For full FP8 K/V and full NVFP4 K/V, the consumer can dispatch each selected layer through existing uniform kernels once every ModelOpt weight-config path resolves the layer map. The current linked PR still needs the uniform-weight configuration fix described above.
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
KV-only search:
Fixed FP8 GEMM PTQ followed by KV AutoQuantize:
Gradient GEMM AutoQuantize followed by independent KV AutoQuantize:
Direct public API usage:
Testing
hf_ptq.pytests: 307 passed;examples/hf_ptqinvocation of the public KV AutoQuant API;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.