fix(models): better support for mixed-precision compressed-tensors NVFP4 - #390
fix(models): better support for mixed-precision compressed-tensors NVFP4 #390Sam-Izdat wants to merge 1 commit into
Conversation
The compressed-tensors iter crashed on mixed-precision NVFP4 checkpoints (FP8 attention/shared-expert + NVFP4 routed experts) and loaded the per-row global scale without reciprocating, producing garbled output. Root cause: the iter and bank builder assumed modelopt's per-expert naming and dequant-side-divisor convention; llm-compressor and the same family use a different naming (`weight_packed` / `weight_global_scale`) and store the quant-side scale instead. Fixes: - Probe the safetensors index for `weight_packed` vs `weight_scale_2` to pick the right reciprocal convention per checkpoint (data, not config claim). - Route compressed-tensors MoE to the modelopt iter when the shared_expert is in the FP8 group, avoiding the FP8+bf16 cat crash. - Extend the NVFP4 expert key regex and bank kind dispatch to cover llm-compressor naming (`weight_packed` / `weight_global_scale` / `input_global_scale`). - `_SharedExpert` refactored to use the `quant_linear` factory, with a new `attn_quant='fp8_pertensor'` dispatch for the mixed-precision path.
|
Thank you for this change. We used it as the base for per-channel compressed-tensors FP8 support, and we found one gap. The new branch in We checked
The value does not change. This value is a routing key, so the checkpoint never reaches The correction is one token: if w.get("group_size") is None and w.get("strategy") in ("tensor", "channel"):This is safe. One more change is necessary with it. We have both changes on a branch with tests. Tell us if you want them in this pull request, and we will send them. We will not open a competing pull request. The full analysis and the measurements are in #252. The change is on a branch, if you want to cherry-pick it.
The commit holds the one-token change and a new test file, Use it, change it, or ignore it, as you prefer. One more observation, for your consideration. It is not a defect in this pull request. The new branch does not consult the checkpoint's
The matcher must differ, though. Neither checkpoint we tested is affected. Neither lists these modules in |
|
Hi -- thanks for this. Before I open a complementary PR, let me flag the overlap and a What overlaps (same base
Why I don't think either patch alone covers the other's models:
Suggestion: rather than merge overlapping diffs, reconcile the MoE-CT dense routing (tagging for visibility; happy to take this to the developer Slack if easier) |
|
@salekseev @Romeriz - Thank you both for feedback and corrections. I agree that we should consolidate, and either way is fine by me. I can fold in your changes if you like, or you can take what you need from here for one of your PRs, and I can close this one -- just let me know. Just a heads up: I'm sitting on an ancient workstation with a 3060 for compute, so that's the extent of the hardware testing I can do, if you need to target more demanding models. |
|
@Sam-Izdat — please don't close this on my account. Two of my four commits turned out not to touch anything you or @Romeriz touch, so I've just opened them as standalone PRs against
I checked rather than assumed: this PR's diff has no The I'm deliberately not weighing in on the routing overlap between you two. I've only exercised this against two checkpoints and I'd be guessing about the exports Romeriz is targeting. That call probably wants @jason-fxz, since as far as I can tell none of the three of us can merge anything here. One correctness thing, because it's easy to lose in a structural discussionRomeriz's point about the index probe is right, and I think it's sharper than it came across. if not idx_path.exists():
return False # single-file or no index -- fall back to safe default
except Exception:
return False # any read error -- don't reciprocate (modelopt default is safer)For a single-file llm-compressor export, Worth considering: you already have the ground truth in your hand at read time. In my commit for the packed shared expert I pick the parts reader because the tensor I'm looking at literally ends in OfferI've got an RTX 4080 SUPER (16 GiB, sm_89) with both |
|
Ran this on hardware. Took @Romeriz's suggestion and read a header instead of an index — more on that below, because there's a reproducible bug in it. #390 loads and serves a real mixed-precision exportI pulled It loads through
Prefill is the fastest of the three. BFCL 92.50 against unsloth's 95.00 is 5 flipped entries out of 200, inside the 8-flip noise floor I've measured on this category, and it's a single run where the others have two or three — so I'd call that unresolved rather than a regression. It's also a different base model (Ornith-1.5, not Qwen3.6), scored with its own tokenizer and chat template. Two notes so nobody over-reads the table. The decode gap is mostly the bf16 The reciprocal probe is wrong for single-file exports, and here's a reproRomeriz was right, and it's worse than "falls back to a default" — the fallback silently picks the opposite convention with no error anywhere. Your own docstring already names the ground truth ("the on-disk tensor naming"); the problem is only that Using the checkpoint above, which is genuinely llm-compressor ( Same tensors both times. Only the index file differs. Worth noting the Where I've landed@Sam-Izdat — #415 and #416 are open against Offer stands on the hardware. I've got this box plus three 35B-class checkpoints on disk and the harnesses wired up, so if either of you wants a branch run through load + needle + BFCL, point me at it. |
Summary
Adds support for compressed-tensors checkpoints that mix per-tensor
FP8 attention/shared-expert projections with NVFP4 routed
experts - a layout produced by
llm-compressorand found in the wild.Without this, such checkpoints either crash on load (FP8 attn gets
torch.cat'd with bf16 weights) or produce garbled output (the dequantkernel multiplies by the per-row global scale, but
llm-compressorstores the QUANT-side scale rather than the DEQUANT-side divisor that
modeloptstores).The fix is in five files, +236/-63 lines. The key idea is to use the
on-disk tensor naming as the ground truth for which convention is
in use (
weight_packed-> llm-compressor,weight_scale_2-> modelopt)rather than relying on the config-side
format: nvfp4-pack-quantizedstring (which both exporters set).
Test models
nvidia/Qwen3.6-35B-A3B-NVFP4primitive-ai/Ornith-1.5-35B-A3B-agentic-NVFP4-FP8Tested with
--moe-backend offload --expert-load parallelon RTX 306012 GB (
nvfp4_backend='triton'). Ornith decoding at ~30-40+ tok/s with minimal tuning.Fixed
Crash on mixed-precision attn+shared-expert — the
compressed-tensors iter doesn't know about FP8. Routing
compressed-tensors MoE to the modelopt iter (which handles FP8
attn) when the shared-expert is in the FP8 group avoids the
cat-of-fp8-and-bf16 crash.
Missing global-reciprocal for llm-compressor NVFP4 experts —
the dequant kernel multiplies by the per-row global;
modeloptstores the DEQUANT-side divisor directly, but
llm-compressorstores the QUANT-side scale that must be reciprocated. A
per-checkpoint flag on the NVFP4 source spec picks the right
convention.
Unknown NVFP4 kind
input_global_scale(parallel build) —the bank dispatch only knew
weight_scale_2(modelopt). Aliasweight_global_scale -> weight_scale_2viaspec.kind_map;skip
input_scale/input_global_scale(activation scales,not bank tensors).
Per-expert naming regex too narrow for parallel build — the
expert key pattern only matched
weight | weight_scale | weight_scale_2. Extended to cover the fullllm-compressorper-expert naming set so the parallel bank builder's
weight_infopopulates correctly.Iter dispatch routed mixed-precision MoE to the
compressed-tensors iter, which holds more GPU state per-shard
than the default iter (caused OOM on 12 GB). Reverted to the
default iter for that case.
The on-disk-naming heuristic for reciprocal was too coarse.
format: nvfp4-pack-quantizedis set by BOTHllm-compressorandmodeloptre-exports. Now probes the safetensors index forweight_packed(llm-compressor signature) vsweight_scale_2(modelopt signature) and picks the convention from the data, not
the config claim.
Files changed (5, +236/-63)
python/freetoken/models/config.py—_nvfp4_global_reciprocalheuristic probing the safetensors index;
ModelConfig.nvfp4_global_reciprocalfieldpython/freetoken/models/nvfp4_banks.py— bank kind dispatchhandles
weight_global_scale(viaspec.kind_map) and skipsactivation scales; serial and parallel paths updated in lockstep
python/freetoken/models/qwen3_5_moe/config.py—_has_moe_expertshelper;
_attn_quantand_expert_quantextended forcompressed-tensors;
dense_quantprobe for FP8 shared-expert;parse_config wiring
python/freetoken/models/qwen3_5_moe/moe.py—_SharedExpertrefactored to use the
quant_linearfactory, withattn_quant='fp8_pertensor'dispatch for the mixed-precisionshared-expert path
python/freetoken/models/qwen3_5_moe/weight.py— per-checkpoint_spec_forwithkind_map;_load_maybe_quantizedand_nvfp4_partsaccept llm-compressor naming;_PT_FP8_FUSEand_CT_NVFP4_FUSEextended for shared-expert gate|up fuse; expanded_NVFP4_EXPERT_KEY_REregexBackwards compatibility
Qwen3.6-35B-A3B-NVFP4(modelopt) is regression-clean.No NVFP4 reciprocal applied (the data shows
weight_scale_2only,which signals the modelopt convention).
exports use) is unchanged in behavior.
Notes
The heuristic that disambiguates
llm-compressorfrommodeloptprobes the safetensors index for a routed-expert tensor's sibling
suffixes. This is a one-time read at model-load time; no per-tensor
overhead. If both
weight_packedandweight_scale_2are absent(e.g. some other compressed-tensors flavor), the heuristic falls
back to the safe default of "no reciprocal" same as
global_reciprocal=Falseupstream.