Skip to content

[Quant] EXL3: load R7 per-(expert, projection) trellis checkpoints - #279

Open
brandonmmusic-max wants to merge 23 commits into
local-inference-lab:dev/gilded-gnosisfrom
brandonmmusic-max:r7/base-228
Open

brandonmmusic-max wants to merge 23 commits into
local-inference-lab:dev/gilded-gnosisfrom
brandonmmusic-max:r7/base-228

Conversation

@brandonmmusic-max

@brandonmmusic-max brandonmmusic-max commented Aug 9, 2026

Copy link
Copy Markdown

[vLLM PR] EXL3: load R7 per-(expert, projection) trellis checkpoints

Target: local-inference-lab/vllm dev/gilded-gnosis
Base: 5ec935796f0afa19e3d8e41888ecc51a6a637528 (the retained #228 shared
EXL3 runtime, as composed into r33)
Delta: 6 files, +1,354 / −14

file +
model_executor/layers/quantization/exl3.py 1256 14
model_executor/models/deepseek_v2.py 22 0
model_executor/model_loader/utils.py 17 0
envs.py 5 0
tests/quantization/test_exl3.py 36 0
tests/quantization/test_exl3_prefill_plan.py 32 0

Companion of the b12x per-projection kernel PR (ABI 8); the two must land
together. Also requires an exllamav3_ext build exporting exl3_moe_r7_fused,
used for layers outside the fused budget and for layers whose K-set exceeds two
values.

Stacking note. This branch sits on the r33-composed base, which is ahead
of public dev/gilded-gnosis. Open it stacked on its prerequisites, or
rebase once they land.

The failure being fixed, as measured on stock r31/r33

Serving brandonmusic/GLM-5.2-EXL3-TR3v4-3.5bpw-MTP78 with --quantization exl3:

  1. The checkpoint's config.json carries quant_method: "modelopt" (producer
    stamp from its NVFP4 lineage). vLLM discards the conflicting embedded config
    — the engine line reads quantization=exl3, quantization_config=None.
  2. Exl3Config.maybe_update_config then hydrates from
    quantization_config.json, which claims only the 375 dense/shared modules
    present in tensor_storage.
  3. The routed experts match nothing: the rank-sliced loader
    (_configure_rank_sliced, exl3.py:877 on stock) requires
    tensor_schema == "model.layers.{L}.mlp.experts.{E}.{proj}.rank{r}.{...}",
    while the R7 checkpoint's tensors are named
    model.layers.{L}.mlp.experts.{E}.{gate|up|down}_proj.{trellis|suh|svh|mcg}
    — no rank component (verified from the safetensors headers).
  4. The MoE layers therefore fall through to
    unquantized_fused_moe_method.py:103 create_weights → torch.empty, which
    attempts BF16 expert allocation and OOMs all four 96 GB GPUs at ~92.7 GiB
    (log attached: r31_stock_test.log, worker tracebacks at lines 240–431).

The stock failure is a silent misroute ending in OOM, not a clean rejection.

Change

  • Config claiming: r7_routed_experts metadata (schema
    r7-complete-v2-checkpoint-v1) claims the expert layers;
    _r7_layer_range_contains scopes it to the declared moe_layers range.
    Schema fields require true JSON integers (no lossy int() coercion), and
    every payload K is checked against the declared k_values set.
  • Weight ingest: per-(expert, projection) tensors are collected as
    exl3_tensors[(expert_id, shard_id)]; per-projection K is derived from the
    tensors' own trailing dims (shape[2] // 16), never trusted from metadata.
  • Tier split (_r7_projection_tiers): exactly two distinct K across all
    three projections → fused path via the b12x three-row descriptor
    (build_projection_tiered_maps); more than two → that layer stays on
    ext.exl3_moe_r7_fused (measured: 4/75 layers on this checkpoint). Empty
    projection tiers allocate from (hidden, intermediate, K) geometry rather
    than borrowing another expert's tensor as a shape template.
  • Fused packing (_prepare_r7_sparkinfer_weights): projection-tight
    [gate | up] slabs filled directly from per-expert sources (each popped as
    consumed — a padded-stack approach OOMed at ~layer 68/75 during bring-up);
    FC2 constructed at its own count; both gate and up counts are threaded to the
    kernel so descriptor bounds are exact; rotations are combined-expert-indexed
    in global order (no tier reordering), padded to the summed tier-slot stride;
    broadcast suh/svh single-row form throughout.
  • CUDA-graph scratch isolation: the shared mixed-Trellis scratch arena is
    keyed by owner and geometry. Keyed on geometry alone, a target layer and
    a same-shape MTP draft layer bind the same mutable route/scratch/output
    tensors into two independently captured graphs; with MTP enabled one graph
    can overwrite state the other is still consuming, giving nondeterministic
    token corruption with no launch failure.
  • Legacy ABI compatibility: the projection-count keywords are passed only
    when the producer supplied the complete paired contract, so legacy mixed
    payloads still launch against a kernel that does not accept them.
  • Memory hygiene: prepare receives a shared never-read ballast (per-layer
    allocation leaked ~200 MiB/layer during bring-up); the pool is released once
    after load in model_loader/utils.py — releasing per layer refragments the
    heap and OOMs; per-expert source storage is dropped as tiers freeze.
  • deepseek_v2.py (separate commit): when num_nextn_predict_layers == 0,
    MTP-layer tensors have no destination module and raise
    KeyError 'layers.78.eh_proj.weight'; they are filtered at load. Inert when
    MTP is on.

Env surface, all registered in envs.py: VLLM_EXL3_R7_FUSED (default on),
VLLM_EXL3_R7_FUSED_LAYERS (fused-layer budget; the fused layout costs
~1.39 GiB/layer vs ~1.03 for the per-expert one), VLLM_EXL3_R7_A1_MIN_ROWS,
VLLM_EXL3_R7_DEBUG, VLLM_EXL3_R7_ROUTE_BLOCK.

Why this shape

The loader adds a reader for an existing checkpoint family without touching any
current path: hybrid_tr3_tail, uniform, and per-expert mixed checkpoints take
exactly the code they take today. All R7 logic is gated on the
r7_routed_experts config block being present.

Testing

Hardware: 4× RTX PRO 6000 Blackwell 96 GB (SM120a), PCIe Gen5 no NVLink,
TP4 + DCP4, Threadripper PRO 9965WX, torch 2.12.0+cu132.

Unit

tests/quantization/test_exl3.py + test_exl3_prefill_plan.py: 64 passed
(run inside the r33 image with these files overlaid; the host workspace cannot
collect vLLM tests because tblib is absent).

Serving

Booted on the r33 image at MAX_MODEL_LEN=262144, TP=4, DCP=4, MTP=3, KV=nvfp4_ds_mla, util=0.955, MAX_BATCHED_TOKENS=2048 (log:
boot/r33_fixed_boot.log, 1,220 lines).

check result
before (stock r33) 4-GPU OOM in unquantized MoE create_weights
after: boot Application startup complete
R7 layers converted 48 unique layer ids, asymmetric membership logged per layer, e.g. model.layers.6.mlp.experts: tier0=K3(180 gate/179 up/25 down) tier1=K4
KV pool 3.76 GiB → 432,384 tokens @ 262,144 MML → 1.65× concurrency
smoke (temperature 0) 4/4 correct, all finish_reason=stop: Marbury judicial-review holding · "Frankfort" · Obama 44th/2009–2017 · 4−1−2=1
output health no NaN, no empty completions
decode 54.5–76.0 tok/s single-stream
log scan 0 ERROR, 0 tracebacks, 0 OOM

Smoke artifact boot/smoke_r33_fixed.json records the full request payload,
served model id, temperature and raw usage for each case.

The ABI-8 correction commit costs essentially nothing in memory: the same stack
before those corrections gave 435,456 KV tokens at 1.66×, so the fail-closed
bounds and owner-keyed scratch arena cost 3,072 tokens (−0.7%).

Throughput, measured with llm_decode_bench (30 s cells, concurrency 1/2/4).
Measured on the previous commit at an identical config, not re-run against
this HEAD; the corrections are memory-safety and validation changes with no
intended throughput effect, but treat these as indicative rather than as this
commit's numbers:

context prefill tok/s decode c1 decode c2 decode c4
0 64.1 91.9 132.6
8k 1300 60.5 90.8 125.4
16k 1297 60.8 88.7 126.0
32k 1280 60.2 89.2 125.1

Provenance of the run

This is not a clean-image result. It is the r33 image plus five
exact-branch-HEAD Python overlays plus an external extension:

image  voipmonitor/vllm:gilded-gnosis-v20-vllmfa13d33-b12x06db0f4-fi1ac6942-cu132-20260809-r33
ext    /opt/exllamav3-r7ext/exllamav3_ext.cpython-312-x86_64-linux-gnu.so
       sha256 e88bc24d2c292a0b69a7ee27bb701557c16e535c9980ee870c931a2b495033bd
       exports exl3_moe_r7_fused = True

Full overlay manifest with per-file SHA-256: boot/mount_manifest_fixed.txt.

Boot-time note: first boot pays ~13 min of serial Triton compiles in the
route-pack prewarm (#126) because every R7 layer has a distinct
(tier0, tier1) expert-count signature; subsequent boots hit the compile
cache. A follow-up could key the prewarm on unique signatures only.

Fidelity: exact-HEAD full-vocab KLD

Teacher-forced 2048-token context, full-vocab (154,880) KL per position against
sha-pinned bf16 reference logits (87f992a6…), 2,047 positions/run, fp8 KV +
bf16 rope, 5 runs. This HEAD, fused path active (48 ABI-8 layers):

stack mean KLD sd n
this HEAD, fused (48 layers, ABI 8) 0.062450 0.001533 5
reference image, extension path (2026-08-04) 0.061282 0.001376 5
same recipe, fusion disabled (2026-08-06) 0.062135 0.001302 5

Welch vs reference: t=1.27, p≈0.20; vs fusion-off: t=0.35, p≈0.73 — the fused
kernel is statistically indistinguishable from both. (For scale: the FC2-bound
bug this PR fixes measured KLD 2.37 on the identical harness, and the pre-r26
lineage floor is ~0.100.) Values:
[0.062530, 0.061651, 0.064910, 0.062349, 0.060810], results
20260809T194958Z-kld-fp8-dcp4. Launcher deviations from the reference
harness are confined to overlay mounts, R7 env forwarding, and a pinned
256 MiB KV pool (kv_cache_memory_bytes) so the eval's full-vocab topk
transient has headroom; the eval script and reference artifact are
byte-identical (audit diff included in the bundle).

Not yet covered

  • No overlapping target/MTP CUDA-graph stress test specifically exercising the
    scratch-isolation fix.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added support for EXL3 quantized checkpoint overlays, including MXFP8 configurations, shared-expert handling, and ignore patterns.
    • Added persistent online encoding caches with configurable modes and automatic recovery from invalid entries.
    • Added configuration options for EXL3 prefill capacity, Trellis encoding, routing, debugging, and cache behavior.
    • Added mixed-bitrate prefill dispatch and runtime warmup support.
  • Bug Fixes

    • Improved handling of disabled speculative prediction layers during model loading.
    • Added safer post-quantization memory cleanup.

malaiwah and others added 21 commits August 7, 2026 16:17
Assisted-by: OpenAI Codex

Signed-off-by: Michel Belleau <michel.belleau@malaiwah.com>
Co-authored-by: OpenAI Codex <noreply@openai.com>
Signed-off-by: Martin Vit <martin@voipmonitor.org>
Co-authored-by: OpenAI Codex <codex@openai.com>
Stock Gilded Gnosis (r31/r33, which carry this PR's base vllm-project#228) cannot serve an
R7 checkpoint such as GLM-5.2-EXL3-TR3v4-3.5bpw-MTP78. Measured on r33: the
checkpoint's quant_method:"modelopt" producer stamp conflicts with
--quantization exl3, so vLLM discards the embedded config
(quantization_config=None); Exl3Config then claims only the dense modules in
tensor_storage; the routed experts match nothing and fall through to the
unquantized MoE method, which BF16-allocates and OOMs all four 96 GB GPUs.
The rank-sliced loader cannot claim them either: it requires the
"...experts.{E}.{proj}.rank{r}..." tensor schema, while R7 tensors are named
"...experts.{E}.{gate|up|down}_proj.{trellis|suh|svh|mcg}" with no rank
component.

This adds the R7 reader on top of vllm-project#228 and reuses its shared-H framework,
rotation broadcasting, mixed prefill/decode runtime, tile/block policies and
prewarm rather than reproducing any of it:

- Claim and strictly validate r7_routed_experts, scoped to the declared
  moe_layers range; the independent hybrid_tr3_tail/MTP78 layer is preserved.
- Ingest per-(expert, projection) tensors; derive each projection's K from the
  payload's own trailing dimension and enforce the K3/K4/K5 contract.
- Two-K layers pack for the b12x per-projection descriptor path (companion
  PR); layers whose K-set is not exactly two stay on ext.exl3_moe_r7_fused.
- Instance-scoped fused-layer budget, charged only after a layer's tiers are
  frozen, so a failed attempt cannot consume budget and the count cannot leak
  across model loads in one process.
- Shared never-read ballast during prepare; per-expert source storage released
  as tiers freeze.

Requires an exllamav3_ext build exposing exl3_moe_r7_fused for the non-fused
R7 layers, and the companion b12x per-projection descriptor PR (ABI 7). The
three land as one unit.

Ruff clean. Boot validation on the exact head is pending and will be reported
against the r33 image rather than any private overlay.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…P is off

Companions to the R7 loader, both conditioned independently of it:

- model_loader/utils.py: release the R7 ballast pool once after every layer's
  weights are processed. The pool is never-read scratch that otherwise stays
  resident for the process lifetime at the direct expense of KV cache;
  releasing it per layer refragments the heap and OOMs the load. Guarded, so
  it is inert unless the EXL3 R7 path allocated it.
- models/deepseek_v2.py: when num_nextn_predict_layers == 0 the MTP layers are
  never built, so their checkpoint tensors are not diverted and reach
  AutoWeightsLoader with no destination, raising
  KeyError 'layers.78.eh_proj.weight'. Skip them at load. Inert when MTP is on.

Ruff clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Booted and smoke-tested on the stock r33 image. Retains the tight
[gate|up] w13 buffer rather than a padded [2, max] stack: the kernel is told
each tier's gate count explicitly (run_mixed_trellis gate_experts), so its
w13 descriptor is sized by gate_count and the up-block base is exact rather
than inferred from a padded extent.

Padding instead was measured at -1.81 GiB of KV headroom at max_model_len
262144 on 4x96 GB, i.e. the engine could not allocate any cache blocks.
With the tight layout: 435,456 KV tokens, 1.66x concurrency.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review follow-up to the R7 loader commit.

- Scratch-arena isolation (correctness). _MIXED_TRELLIS_BUFFERS was
  keyed on launch geometry alone, so a target layer and a same-shape
  MTP draft layer bound the same mutable route/scratch/output tensors
  into two independently captured CUDA graphs. With MTP enabled, one
  graph could overwrite state the other was still consuming, giving
  nondeterministic token corruption with no launch failure. The owner
  token now participates in the buffer key.
- Legacy ABI-6 compatibility. The projection keywords were passed
  unconditionally, so a legacy mixed payload carrying no R7 counts hit
  TypeError before launch. They are now passed only when the producer
  supplied the complete paired contract.
- Empty projection tiers. A K present only in up or only in down
  borrowed expert 0's tensor as a shape template; that expert can
  belong to the other K or already have been consumed. Slabs are now
  allocated from (hidden, intermediate, K) geometry.
- Strict R7 metadata. Schema fields required true JSON integers
  instead of lossy int() coercion, and each payload K is checked
  against the declared k_values set.
- Register the five consumed VLLM_EXL3_R7_* variables in envs.py; the
  engine warned they were unknown.
- Support the non-dataclass launch object used by the retained API
  test, and sort the utils.py import block.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cac49e5f-72cb-4b39-83cf-4907ecfa11db

📥 Commits

Reviewing files that changed from the base of the PR and between bd38714 and 7b2d831.

📒 Files selected for processing (2)
  • vllm/model_executor/layers/quantization/exl3.py
  • vllm/model_executor/models/deepseek_v2.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • vllm/model_executor/models/deepseek_v2.py

📝 Walkthrough

Walkthrough

The PR adds EXL3 online MXFP8 overlay support, deterministic rank-local encoding caches, mixed-Trellis prefill planning and warmup, expanded validation coverage, post-quantization cleanup, and filtering for disabled DeepSeek MTP weights.

Changes

EXL3 online quantization

Layer / File(s) Summary
Overlay contract and configuration
vllm/config/quantization.py, vllm/envs.py, docs/features/quantization/online.md, tests/quantization/test_quantization_config_args.py
EXL3 MXFP8 checkpoint overlays are validated, configured, documented, and tested.
Deterministic online encoding cache
vllm/model_executor/layers/quantization/exl3_online_cache.py, tests/quantization/test_exl3_online_cache.py, tests/quantization/test_exl3.py
Model and encoder identities, cache modes, tensor validation, atomic persistence, and cache-hit behavior are covered.
Mixed-Trellis metadata and preparation
tests/quantization/test_exl3.py
Shared-H layouts, broadcast rotation rows, mixed-bitrate preparation, ownership, dispatch tiers, and R7 schema validation are covered.
Prefill capacity and warmup integration
tests/quantization/test_exl3_prefill_plan.py, tests/quantization/test_exl3_warmup.py, vllm/model_executor/warmup/kernel_warmup.py, vllm/model_executor/model_loader/utils.py
Capacity-bounded prefill slicing, routing, runtime caches, capture behavior, route-pack warmup, and post-quantization cleanup are covered or integrated.

DeepSeek weight loading

Layer / File(s) Summary
Disabled MTP weight filtering
vllm/model_executor/models/deepseek_v2.py
Checkpoint weights for disabled MTP layers are skipped before normal weight loading.

Estimated code review effort: 5 (Critical) | ~90+ minutes

Sequence Diagram(s)

sequenceDiagram
  participant EXL3WeightLoader
  participant load_or_quantize
  participant EXL3Encoder
  participant OnlineCache
  EXL3WeightLoader->>load_or_quantize: request encoded tensors
  load_or_quantize->>OnlineCache: load and validate cache entry
  OnlineCache-->>load_or_quantize: cache hit or cache miss
  load_or_quantize->>EXL3Encoder: encode weights on cache miss
  EXL3Encoder-->>load_or_quantize: encoded tensors
  load_or_quantize->>OnlineCache: persist cache result
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 24.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: loading EXL3 R7 per-(expert, projection) trellis checkpoints.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (3)
tests/quantization/test_exl3_prefill_plan.py (1)

64-121: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add Google-style docstrings to the new Python callables.

  • tests/quantization/test_exl3_prefill_plan.py#L64-L121: Document the fake API methods and their routing/output behavior.
  • tests/quantization/test_exl3_prefill_plan.py#L152-L168: Document _make_mixed_layer and its returned layer configuration.
  • tests/quantization/test_exl3_prefill_plan.py#L248-L437: Document each new test function with its validated behavior.
  • tests/quantization/test_exl3_warmup.py#L12-L55: Document the helper and each new test function.

As per coding guidelines, Python code must use Google-style docstrings with Args:, Returns:, and Raises: sections instead of Sphinx fields.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/quantization/test_exl3_prefill_plan.py` around lines 64 - 121, Add
Google-style docstrings to every new callable in
tests/quantization/test_exl3_prefill_plan.py lines 64-121, documenting the fake
API methods and their routing/output behavior; lines 152-168, documenting
_make_mixed_layer and its returned layer configuration; lines 248-437,
documenting each new test and its validated behavior; and
tests/quantization/test_exl3_warmup.py lines 12-55, documenting the helper and
each new test. Include Args, Returns, and Raises sections where applicable,
using Google-style conventions rather than Sphinx fields.

Source: Coding guidelines

vllm/config/quantization.py (1)

161-172: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider expressing the EXL3 entry in the literal.

The post-construction mutation at Line 172 separates one mapping entry from the rest. A single dict literal keeps the supported-weight contract visible in one place.

♻️ Optional restructure
-_CHECKPOINT_ONLINE_OVERLAY_WEIGHTS = {
-    name: frozenset({kMxfp8Dynamic, kFp8Static128BlockSym})
-    for name in {
-        "modelopt",
-        "modelopt_fp4",
-        "modelopt_mxfp8",
-        "modelopt_mixed",
-        "mxfp4",
-        "nvfp4_nf3_hybrid",
-    }
-}
-_CHECKPOINT_ONLINE_OVERLAY_WEIGHTS["exl3"] = frozenset({kMxfp8Dynamic})
+_CHECKPOINT_ONLINE_OVERLAY_WEIGHTS: dict[str, frozenset[QuantKey]] = {
+    **{
+        name: frozenset({kMxfp8Dynamic, kFp8Static128BlockSym})
+        for name in (
+            "modelopt",
+            "modelopt_fp4",
+            "modelopt_mxfp8",
+            "modelopt_mixed",
+            "mxfp4",
+            "nvfp4_nf3_hybrid",
+        )
+    },
+    "exl3": frozenset({kMxfp8Dynamic}),
+}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@vllm/config/quantization.py` around lines 161 - 172, Update
_CHECKPOINT_ONLINE_OVERLAY_WEIGHTS to include the "exl3" mapping directly in the
dictionary literal, preserving its frozenset({kMxfp8Dynamic}) value, and remove
the post-construction assignment.
tests/quantization/test_exl3_online_cache.py (1)

168-180: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert that a shard change also changes the identity.

The test name states that the identity tracks metadata and shards. The body writes shard at Line 174 and never modifies it, so only the config.json marker path is exercised. Shard tracking uses size and mtime_ns in resolve_model_identity, which is the weaker half of the fingerprint and the part whose regression would cause stale cached weights to be reused.

💚 Proposed additional assertion
     before = resolve_model_identity(str(model))
     config.write_text('{"model_type":"changed"}', encoding="utf-8")
     after = resolve_model_identity(str(model))
 
     assert before != after
+
+    # A shard rewrite of a different length must also change the identity.
+    shard.write_bytes(b"different weight payload")
+    assert resolve_model_identity(str(model)) != after
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/quantization/test_exl3_online_cache.py` around lines 168 - 180, Extend
test_local_model_identity_tracks_metadata_and_shards to modify the existing
shard after the initial resolve_model_identity call, then resolve the identity
again and assert it differs from the original. Preserve the existing config
metadata assertion while ensuring the shard change exercises size or mtime_ns
tracking.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/quantization/test_exl3.py`:
- Around line 229-233: Update the cleanup around _load_exl3_online_quantizer to
permanently remove the synthetic _vllm_exl3_encoder module and its submodules
from sys.modules, rather than using monkeypatch.delitem, so teardown cannot
restore them. Place cleanup in a finally block surrounding the pytest.raises
assertion so it also runs when the expected exception assertion fails.

In `@vllm/model_executor/layers/quantization/exl3_online_cache.py`:
- Around line 182-194: Update cache_root() to use envs.VLLM_CACHE_ROOT for the
fallback cache directory instead of reading VLLM_CACHE_ROOT directly with a
hardcoded ~/.cache/vllm default, preserving the configured
VLLM_EXL3_ONLINE_CACHE_DIR override and XDG-aware default resolution.
- Around line 318-339: Update the cache-write flow around _save so any
publish/serialization failure is caught separately and returns the already
encoded result without aborting model loading; keep the existing OSError
fallback for cache-unavailable conditions. Add `# noqa: BLE001` with a brief
reason to both intentional broad exception handlers, including the existing
handler near the cache-load path and the handler around invalid cache
replacement.

In `@vllm/model_executor/models/deepseek_v2.py`:
- Around line 2198-2199: Expand the _skip_disabled_mtp_weight docstring with
Google-style Args: entries for config and name, and a Returns: entry describing
the boolean result; preserve the existing summary and document only these
parameters and return value.
- Around line 2201-2205: Normalize or reject malformed num_nextn_predict_layers
and num_hidden_layers before weight filtering begins, including the paths in
get_spec_layer_idx_from_weight_name() and _skip_disabled_mtp_weight(). Ensure
None, non-numeric, and invalid values cannot reach comparisons, direct
assignments, or iteration, while preserving the existing behavior for valid
positive layer counts.

---

Nitpick comments:
In `@tests/quantization/test_exl3_online_cache.py`:
- Around line 168-180: Extend
test_local_model_identity_tracks_metadata_and_shards to modify the existing
shard after the initial resolve_model_identity call, then resolve the identity
again and assert it differs from the original. Preserve the existing config
metadata assertion while ensuring the shard change exercises size or mtime_ns
tracking.

In `@tests/quantization/test_exl3_prefill_plan.py`:
- Around line 64-121: Add Google-style docstrings to every new callable in
tests/quantization/test_exl3_prefill_plan.py lines 64-121, documenting the fake
API methods and their routing/output behavior; lines 152-168, documenting
_make_mixed_layer and its returned layer configuration; lines 248-437,
documenting each new test and its validated behavior; and
tests/quantization/test_exl3_warmup.py lines 12-55, documenting the helper and
each new test. Include Args, Returns, and Raises sections where applicable,
using Google-style conventions rather than Sphinx fields.

In `@vllm/config/quantization.py`:
- Around line 161-172: Update _CHECKPOINT_ONLINE_OVERLAY_WEIGHTS to include the
"exl3" mapping directly in the dictionary literal, preserving its
frozenset({kMxfp8Dynamic}) value, and remove the post-construction assignment.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 01fa2364-7111-4615-bf13-476a40074167

📥 Commits

Reviewing files that changed from the base of the PR and between e2666d9 and bd38714.

📒 Files selected for processing (13)
  • docs/features/quantization/online.md
  • tests/quantization/test_exl3.py
  • tests/quantization/test_exl3_online_cache.py
  • tests/quantization/test_exl3_prefill_plan.py
  • tests/quantization/test_exl3_warmup.py
  • tests/quantization/test_quantization_config_args.py
  • vllm/config/quantization.py
  • vllm/envs.py
  • vllm/model_executor/layers/quantization/exl3.py
  • vllm/model_executor/layers/quantization/exl3_online_cache.py
  • vllm/model_executor/model_loader/utils.py
  • vllm/model_executor/models/deepseek_v2.py
  • vllm/model_executor/warmup/kernel_warmup.py

Comment on lines +229 to +233
with pytest.raises(RuntimeError, match="has no quantize_exl3"):
exl3_module._load_exl3_online_quantizer()
for name in tuple(exl3_module.sys.modules):
if name == "_vllm_exl3_encoder" or name.startswith("_vllm_exl3_encoder."):
monkeypatch.delitem(exl3_module.sys.modules, name, raising=False)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

monkeypatch.delitem restores the synthetic modules at teardown.

_load_exl3_online_quantizer inserts _vllm_exl3_encoder and three submodules into sys.modules before it raises. Lines 231-233 remove them with monkeypatch.delitem, but monkeypatch records the removal and re-inserts the original values when the test ends. The synthetic modules therefore survive into the rest of the session, and their __path__ points at a deleted tmp_path.

The cleanup also does not run if the pytest.raises assertion at Line 229 fails.

💚 Proposed fix
-    with pytest.raises(RuntimeError, match="has no quantize_exl3"):
-        exl3_module._load_exl3_online_quantizer()
-    for name in tuple(exl3_module.sys.modules):
-        if name == "_vllm_exl3_encoder" or name.startswith("_vllm_exl3_encoder."):
-            monkeypatch.delitem(exl3_module.sys.modules, name, raising=False)
+    try:
+        with pytest.raises(RuntimeError, match="has no quantize_exl3"):
+            exl3_module._load_exl3_online_quantizer()
+    finally:
+        for name in tuple(exl3_module.sys.modules):
+            if name == "_vllm_exl3_encoder" or name.startswith(
+                "_vllm_exl3_encoder."
+            ):
+                del exl3_module.sys.modules[name]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
with pytest.raises(RuntimeError, match="has no quantize_exl3"):
exl3_module._load_exl3_online_quantizer()
for name in tuple(exl3_module.sys.modules):
if name == "_vllm_exl3_encoder" or name.startswith("_vllm_exl3_encoder."):
monkeypatch.delitem(exl3_module.sys.modules, name, raising=False)
try:
with pytest.raises(RuntimeError, match="has no quantize_exl3"):
exl3_module._load_exl3_online_quantizer()
finally:
for name in tuple(exl3_module.sys.modules):
if name == "_vllm_exl3_encoder" or name.startswith(
"_vllm_exl3_encoder."
):
del exl3_module.sys.modules[name]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/quantization/test_exl3.py` around lines 229 - 233, Update the cleanup
around _load_exl3_online_quantizer to permanently remove the synthetic
_vllm_exl3_encoder module and its submodules from sys.modules, rather than using
monkeypatch.delitem, so teardown cannot restore them. Place cleanup in a finally
block surrounding the pytest.raises assertion so it also runs when the expected
exception assertion fails.

Comment on lines +182 to +194
def cache_root() -> Path:
"""Return the trusted online-weight cache directory.

Cached payloads become model weights after schema validation. The directory
must therefore be writable only by the serving user and trusted to the same
degree as the source checkpoint.
"""

configured = os.getenv("VLLM_EXL3_ONLINE_CACHE_DIR")
if configured and configured.strip():
return Path(configured).expanduser()
vllm_root = Path(os.getenv("VLLM_CACHE_ROOT", "~/.cache/vllm")).expanduser()
return vllm_root / "exl3_online"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use envs.VLLM_CACHE_ROOT instead of reading the raw environment variable.

vllm.envs.VLLM_CACHE_ROOT resolves its default through get_default_cache_root(), which honors XDG_CACHE_HOME. Line 193 hardcodes ~/.cache/vllm. If a deployment sets XDG_CACHE_HOME and leaves VLLM_CACHE_ROOT unset, this cache lands outside the directory every other vLLM cache uses.

🔧 Proposed fix
+import vllm.envs as envs
+
...
     configured = os.getenv("VLLM_EXL3_ONLINE_CACHE_DIR")
     if configured and configured.strip():
         return Path(configured).expanduser()
-    vllm_root = Path(os.getenv("VLLM_CACHE_ROOT", "~/.cache/vllm")).expanduser()
-    return vllm_root / "exl3_online"
+    return Path(envs.VLLM_CACHE_ROOT) / "exl3_online"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def cache_root() -> Path:
"""Return the trusted online-weight cache directory.
Cached payloads become model weights after schema validation. The directory
must therefore be writable only by the serving user and trusted to the same
degree as the source checkpoint.
"""
configured = os.getenv("VLLM_EXL3_ONLINE_CACHE_DIR")
if configured and configured.strip():
return Path(configured).expanduser()
vllm_root = Path(os.getenv("VLLM_CACHE_ROOT", "~/.cache/vllm")).expanduser()
return vllm_root / "exl3_online"
import vllm.envs as envs
def cache_root() -> Path:
"""Return the trusted online-weight cache directory.
Cached payloads become model weights after schema validation. The directory
must therefore be writable only by the serving user and trusted to the same
degree as the source checkpoint.
"""
configured = os.getenv("VLLM_EXL3_ONLINE_CACHE_DIR")
if configured and configured.strip():
return Path(configured).expanduser()
return Path(envs.VLLM_CACHE_ROOT) / "exl3_online"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@vllm/model_executor/layers/quantization/exl3_online_cache.py` around lines
182 - 194, Update cache_root() to use envs.VLLM_CACHE_ROOT for the fallback
cache directory instead of reading VLLM_CACHE_ROOT directly with a hardcoded
~/.cache/vllm default, preserving the configured VLLM_EXL3_ONLINE_CACHE_DIR
override and XDG-aware default resolution.

Comment on lines +318 to +339
try:
path.parent.mkdir(parents=True, exist_ok=True)
lock = filelock.FileLock(f"{path}.lock")
with lock:
if path.is_file():
try:
return _to_device(_load(path, key), device)
except Exception as exc:
logger.warning(
"Replacing invalid online EXL3 cache %s: %s", path, exc
)
path.unlink(missing_ok=True)
result = _quantize(key, quantize, path=path)
_save(path, key, result)
return result
except OSError as exc:
logger.warning(
"Online EXL3 cache is unavailable at %s; encoding without cache: %s",
path,
exc,
)
return _quantize(key, quantize, path=None)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Do not let a cache-write failure abort model loading.

The outer handler catches only OSError. The encoded tensors already exist when _save runs at Line 331. If _save raises anything else, for example a serialization error from save_file, the exception escapes and fails model load even though the payload is usable. Catch the publish failure separately and return the encoded result.

The two except Exception handlers at Lines 325 and 312 are intentional cache-degradation paths. Add # noqa: BLE001 with a short reason so Ruff stays clean.

🛡️ Proposed fix
     try:
         path.parent.mkdir(parents=True, exist_ok=True)
         lock = filelock.FileLock(f"{path}.lock")
         with lock:
             if path.is_file():
                 try:
                     return _to_device(_load(path, key), device)
-                except Exception as exc:
+                except Exception as exc:  # noqa: BLE001 - any decode failure re-encodes
                     logger.warning(
                         "Replacing invalid online EXL3 cache %s: %s", path, exc
                     )
                     path.unlink(missing_ok=True)
             result = _quantize(key, quantize, path=path)
-            _save(path, key, result)
-            return result
+            try:
+                _save(path, key, result)
+            except Exception as exc:  # noqa: BLE001 - publishing is best-effort
+                logger.warning(
+                    "Failed to publish online EXL3 cache %s: %s", path, exc
+                )
+                return Exl3OnlineCacheResult(
+                    result.tensors, result.proxy_error, result.hit, None
+                )
+            return result
     except OSError as exc:

Apply the same # noqa: BLE001 comment to Line 312.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
try:
path.parent.mkdir(parents=True, exist_ok=True)
lock = filelock.FileLock(f"{path}.lock")
with lock:
if path.is_file():
try:
return _to_device(_load(path, key), device)
except Exception as exc:
logger.warning(
"Replacing invalid online EXL3 cache %s: %s", path, exc
)
path.unlink(missing_ok=True)
result = _quantize(key, quantize, path=path)
_save(path, key, result)
return result
except OSError as exc:
logger.warning(
"Online EXL3 cache is unavailable at %s; encoding without cache: %s",
path,
exc,
)
return _quantize(key, quantize, path=None)
try:
path.parent.mkdir(parents=True, exist_ok=True)
lock = filelock.FileLock(f"{path}.lock")
with lock:
if path.is_file():
try:
return _to_device(_load(path, key), device)
except Exception as exc: # noqa: BLE001 - any decode failure re-encodes
logger.warning(
"Replacing invalid online EXL3 cache %s: %s", path, exc
)
path.unlink(missing_ok=True)
result = _quantize(key, quantize, path=path)
try:
_save(path, key, result)
except Exception as exc: # noqa: BLE001 - publishing is best-effort
logger.warning(
"Failed to publish online EXL3 cache %s: %s", path, exc
)
return Exl3OnlineCacheResult(
result.tensors, result.proxy_error, result.hit, None
)
return result
except OSError as exc:
logger.warning(
"Online EXL3 cache is unavailable at %s; encoding without cache: %s",
path,
exc,
)
return _quantize(key, quantize, path=None)
🧰 Tools
🪛 Ruff (0.16.1)

[warning] 325-325: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@vllm/model_executor/layers/quantization/exl3_online_cache.py` around lines
318 - 339, Update the cache-write flow around _save so any publish/serialization
failure is caught separately and returns the already encoded result without
aborting model loading; keep the existing OSError fallback for cache-unavailable
conditions. Add `# noqa: BLE001` with a brief reason to both intentional broad
exception handlers, including the existing handler near the cache-load path and
the handler around invalid cache replacement.

Source: Linters/SAST tools

Comment thread vllm/model_executor/models/deepseek_v2.py Outdated
Comment on lines +2201 to +2205
try:
nextn = int(getattr(config, "num_nextn_predict_layers", 0) or 0)
hidden = int(getattr(config, "num_hidden_layers", 0) or 0)
except (TypeError, ValueError):
return False

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Expect: all configuration paths normalize these fields to integers
# or reject invalid values before DeepseekV2Model.load_weights runs.
rg -n -C 6 \
  'num_nextn_predict_layers|num_hidden_layers|get_spec_layer_idx_from_weight_name|_skip_disabled_mtp_weight' \
  vllm tests --glob '*.py'

Repository: local-inference-lab/vllm

Length of output: 50381


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the helpers and construction/config paths for num_nextn_predict_layers
# and num_hidden_layers in the relevant files.
sed -n '2160,2225p' vllm/model_executor/models/deepseek_v2.py
printf '\n--- deepseekV2 class config assignment ---\n'
sed -n '1570,1640p' vllm/model_executor/models/deepseek_v2.py

printf '\n--- focused search for field assignments/normalization ---\n'
rg -n -C 3 \
  'num_nextn_predict_layers|num_hidden_layers' \
  vllm/model_executor/models/deepseek_v2.py \
  vllm/transformers_utils/configs \
  tests --glob '*.py' \
  | sed -n '1,220p'

printf '\n--- Python behavior probe for malformed inputs ---\n'
python3 - <<'PY'
class C:
    num_nextn_predict_layers = None
    num_hidden_layers = None

config = C()
try:
    try:
        nextn = int(getattr(config, "num_nextn_predict_layers", 0) or 0)
        hidden = int(getattr(config, "num_hidden_layers", 0) or 0)
    except (TypeError, ValueError):
        result_skip = False
    spec = int(getattr(config, "num_hidden_layers", 0) or 0)
    print("skip_disabled=", result_skip)
    print("spec_layer=", spec)
except Exception as e:
    print(type(e).__name__, str(e))
PY

Repository: local-inference-lab/vllm

Length of output: 22496


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
values = [None, "abc", 2.3, [1], {"x": 1}]

for value in values:
    class C:
        num_nextn_predict_layers = value
        num_hidden_layers = value

    config = C()
    try:
        nextn = int(getattr(config, "num_nextn_predict_layers", 0) or 0)
        hidden = int(getattr(config, "num_hidden_layers", 0) or 0)
        print("skip_disabled_normalization value=", value, "result=True nextn=", nextn, "hidden=", hidden)
    except Exception as e:
        print("skip_disabled_normalization value=", value, "exception=", type(e).__name__, str(e))

    try:
        layer_idx = config.num_hidden_layers
        spec = None
        while nextn_placeholder := False:
            spec = "placeholder"
            print("spec_layer_reached loop")
        print("spec_layer_is_None value=", value, "hasattr=", hasattr(config, "num_nextn_predict_layers"), "compares true condition=", (hasattr(config, "num_nextn_predict_layers") and config.num_nextn_predict_layers > 0))
    except Exception as e:
        print("spec_layer_getattr_comparison value=", value, "exception=", type(e).__name__, str(e))
PY

printf '\n--- config construction entrypoints ---\n'
rg -n -C 4 'from_pretrained|load_config|model_config.*hf_config|PretrainedConfig|auto_from_pretrained' vllm tests --glob '*.py' \
  | rg -n -C 3 'num_nextn_predict_layers|num_hidden_layers|hf_config|model_config' \
  | sed -n '1,220p'

Repository: local-inference-lab/vllm

Length of output: 17219


Handle malformed num_nextn_predict_layers/num_hidden_layers consistently.

get_spec_layer_idx_from_weight_name() compares num_nextn_predict_layers > 0 and
assigns config.num_hidden_layers directly before iterating, while
_skip_disabled_mtp_weight() only handles malformed values in the weight skip branch.
A None field can pass the skip guard and then raise TypeError in weight filtering.
Normalize or reject these config fields before this weight-filtering path runs.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@vllm/model_executor/models/deepseek_v2.py` around lines 2201 - 2205,
Normalize or reject malformed num_nextn_predict_layers and num_hidden_layers
before weight filtering begins, including the paths in
get_spec_layer_idx_from_weight_name() and _skip_disabled_mtp_weight(). Ensure
None, non-numeric, and invalid values cannot reach comparisons, direct
assignments, or iteration, while preserving the existing behavior for valid
positive layer counts.

Review follow-up (PR vllm-project#279): Google-style Args/Returns sections.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The online-quantizer import pulls exllamav3_ext into sys.modules by its
canonical name from the encoder package's own tree. When an explicit
VLLM_EXL3_EXT_PATH build is configured (required for the R7 fused path),
that stock import wins the module cache and exl3_moe_r7_fused resolves
missing at graph preparation. Load the configured extension first; it is
a superset of the stock symbols, so the encoder and the fused path share
one module.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@brandonmmusic-max

Copy link
Copy Markdown
Author

While converting my R7 checkpoint to BF16 shared experts (the proposed layout from the discord chat — shared gate/up/down left in BF16 so the runtime’s ONLINE_QUANT=exl3-b6 can encode one merged gate_up_proj K6 payload at load), I hit a boot failure that turns out to be the first-ever collision between two features of this PR: RuntimeError: R7 CUDA-graph execution requires exl3_moe_r7_fused;
the extension and vLLM patches must be installed together

The extension was installed. The problem I saw was import order: the online encoder’s internals import exllamav3_ext by its canonical name, and during load the encoder runs first — so the stock build next to the encoder tree wins sys.modules. It passes the loader’s exl3_gemm sanity check but doesn’t export exl3_moe_r7_fused. When the R7 path later calls _load_exl3_ext(), importlib.import_module("exllamav3_ext") returns the cached stock module and the VLLM_EXL3_EXT_PATH build silently never loads. Meanwhile the online encoder itself was working fine — merged shared_experts.gate_up_proj K6 encodes streaming by at ~3e-4 proxy error — which is what made the “extension not installed” message so misleading.

I don’t think anyone could have hit this before: my fully-prequantized R7 checkpoint never invoked the online encoder,. A BF16-shared-expert R7 checkpoint is the first configuration that needs both in one process. First boot, instant repro. So i hadn’t encountered that error until first boot to update this checkpoint as suggested.

Fix in 7b2d831: when VLLM_EXL3_EXT_PATH is set, _load_exl3_online_quantizer() loads the pinned extension first, so it owns the canonical module name and the encoder’s internal import lands on the same module — the pinned build is a strict superset of the stock symbols. Deliberately not loading both .sos under different names, since that risks duplicate torch-op registration. I think its the same failure family as the earlier in-image-ext-wins-the-cache bug fixed in the #190 lineage.

With the fix, the merged-shared-expert layout boots clean on this branch: 48 fused R7 layers + online-K6 merged shared experts in one process. Full KLD + throughput numbers for the new layout coming to the checkpoint card once testing finishes.

I just wanted to update it here for clarity before any merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants