Repository navigation
fix(lora): keep fused LoRA weights in the base weight's dtype - #2527
Merged
Merged
Conversation
`fuse_lora_weights_into` added the f32 LoRA delta straight onto the base weight. MLX promotes bf16 + f32 to f32, so on a bf16/fp16 checkpoint every projection the adapter touched was stored as f32: twice the bytes, and every activation downstream of it became f32 as well. `--lora-fuse` and `mlxcel generate --adapter`, which exist to remove the adapter's decode cost, made decode slower than the unfused path instead. Cast the delta to the base weight's dtype before the add, as mlx-lm's `LoRALinear.fuse` and the embedding fusion (`embedding.rs`) already do. Qwen3-1.7B bf16 with a rank-8 adapter on 16 layers x 7 projections, M4 Pro: - `mlxcel generate --adapter`: decode 27 -> 64 tok/s (base 63), peak footprint 8.6 -> 5.0 GB (base 5.5) - `mlxcel-server --lora --lora-fuse`: decode 27 -> 60 tok/s (base 60)
Member
|
Hardening review for PR
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--lora-fuseandmlxcel generate --adapterstored every fused projection of a bf16/fp16 checkpoint as f32.fuse_lora_weights_intoadded the f32 delta fromcompute_lora_deltastraight onto the base weight, and MLX promotes bf16 + f32 to f32. The fused weights took twice the bytes and every activation downstream of them became f32. The mode that is meant to remove the adapter's decode cost ended up slower than the unfused path.This PR casts the delta to the base weight's dtype before the add. mlx-lm's
LoRALinear.fuse(delta = (...).astype(weight.dtype)) and mlxcel's own embedding fusion (src/lora/embedding.rs) already do this.Related issues
No issue filed. Found while measuring LoRA serving. Follow-up to #2446, which made the delta f32 explicitly but left the add uncast.
Type of change
fix— bug fixperf— performance improvement (before/after numbers below)Measurements
Qwen/Qwen3-1.7B(bf16) with a rank-8 adapter trained bymlx_lm lora(16 layers x 7 projections, f32 adapter tensors) on an Apple M4 Pro 48 GB. Decode in tok/s; peak footprint from/usr/bin/time -l.mlxcel generate --adapterdecodemlxcel generate --adapterpeak footprintmlxcel-server --lora --lora-fusedecode (/completiontimings, 256 tokens)For comparison, the unfused
--lorapath on the same server runs at 54–55 tok/s.Parity with mlx-lm
mlx_lm fuse(mlx-lm 0.31.3). Each fused weight was then compared withW + (a @ b).T * scalecast to bf16, which is what this PR computes.lora_bbefore the matmul, and mlxcel scales after.--adapterand the mlx-lm-fused checkpoint gave the same text on 3. The 4th diverged at an early token, and both continuations stayed in the adapter's style.Before this PR the fused weights were not bf16 at all.
Test plan
cargo fmt --all -- --checkcargo clippy -p mlxcel --lib --tests --features metal,accelerate -- -D warningscargo test --profile test-fast --features metal,accelerate -p mlxcel --lib lora:: -- --test-threads=1: 71 passed, including the new testa_fused_weight_keeps_the_base_dtype, run against bf16, fp16 and f32 bases. It fails on e160079 (left: 10(f32),right: 12(bf16)) and passes with the fix.cargo test --workspace --profile test-fast --features metal,accelerate --no-fail-fast -- --test-threads=1: all pass except 6models::switch_layers::sorted_gather_{layout,history}_testscases. Those fail identically onmain(e160079) without this change. On this M4 Pro each one reportsoff the f64 reference by 1.017e-2 relative (bf16 ulp is 3.906e-3)fromassert_matches_referenceinsrc/models/switch_layers_sorted_gather_test_support.rs. They are unrelated to LoRA, and I have not root-caused them.Qwen/Qwen3-1.7Bbf16 plus an mlx-lm LoRA adapter (numbers above)