From 2ef98d382e0e38603b18854cc7b2451859bdfd3c Mon Sep 17 00:00:00 2001 From: Ricardo-M-L <69202550+Ricardo-M-L@users.noreply.github.com> Date: Tue, 5 May 2026 18:48:04 +0800 Subject: [PATCH] fix: use % 8 instead of // 8 in FP8 weight shape check (#5243) * fix: preserve bf16 GGUF file when explicitly requested in quantization list When users request multiple quantization methods including the base format (e.g., ["q4_k_m", "bf16"]), the bf16 GGUF serves as both the intermediate conversion and a user-requested output. The cleanup step unconditionally deleted this file, losing the explicitly requested bf16 output. Only delete the intermediate base GGUF when the user did not request it. Fixes #4932 Co-Authored-By: Claude Opus 4.6 (1M context) * fix: keep reverse() outside conditional deletion to preserve VLM ordering Address review feedback: the reverse() call must always execute when quants_created is True to maintain correct [text_model, mmproj] ordering for VLMs. Only the file deletion should be conditional on whether the user requested the base format. Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Ricardo-M-L * fix: move preserved base GGUF away from list boundaries for correct example commands Address review from @Datta0: when the base format (e.g. bf16) is kept in all_saved_locations, it could end up at [-1], causing the VLM example command to use bf16 as --mmproj instead of the actual projector file. Move the preserved base file to index 1 (after the primary quantized model, before mmproj) so [0] and [-1] remain correct for example commands. Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Ricardo-M-L * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix: use % 8 instead of // 8 in FP8 weight shape check weight.shape[X] // 8 != 0 is True for any non-zero dimension, causing incorrect fallback to dequantization for small weights. Using % 8 correctly checks non-divisibility: weights not divisible by 8 should dequantize, while those with % 8 == 0 stay on the fast kernel path. * Preserve sharded base GGUF files during cleanup convert_to_gguf can return multiple base text shards plus an mmproj entry when llama.cpp splits the output. The previous cleanup only removed/repositioned base_gguf=initial_files[0]: - when the base format is NOT in quantization_method, sibling shards were left both in all_saved_locations and on disk as orphans - when the base IS preserved, the reverse + insert(1, base_gguf) step left a sibling base shard at all_saved_locations[-1] for VLMs, so the example llama-mtmd-cli command ended up with --mmproj pointing at a text shard instead of the projector Treat every initial file whose basename does not contain "-mmproj" as part of the base set, then remove/unlink or reposition all of them together. Drop the redundant frozenset() construction at both call sites and the dead `base_gguf in all_saved_locations` clause in the reorder guard. * Apply bias in FP8 dequant fallback and dedupe full-precision flag unsloth/kernels/fp8.py: FbgemmFp8Linear_matmul.forward had a dequant fallback that called torch_matmul without adding bias. The fast row-wise branch and the block FP8 branch both apply `output = output + bias if bias is not None else output` immediately after the matmul; the fallback now matches. This silently dropped bias for any FP8 layer routed to the fallback (Qwen 2.5 VL gate/up_proj 3420x1280, transposed-weight backward dispatch, and the small-shape cases newly routed here by the recent `% 8` divisibility fix). unsloth/save.py: preserved_base inside the cleanup block and want_full_precision below it computed the identical expression `first_conversion in quantization_method`. Hoist want_full_precision above the cleanup block, reuse it for the not-preserved deletion and the preserved reposition, and assign True directly in the GPT-OSS branch. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Signed-off-by: Ricardo-M-L Co-authored-by: Claude Opus 4.6 (1M context) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han Co-authored-by: Lee Jackson <130007945+Imagineer99@users.noreply.github.com> --- unsloth/kernels/fp8.py | 3 ++- unsloth/save.py | 26 ++++++++++++++++++++------ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/unsloth/kernels/fp8.py b/unsloth/kernels/fp8.py index a57f4ffb64..f704d4e3c4 100644 --- a/unsloth/kernels/fp8.py +++ b/unsloth/kernels/fp8.py @@ -432,13 +432,14 @@ class FbgemmFp8Linear_matmul(torch.autograd.Function): elif ( weight.shape[0] != weight_scale.shape[0] and weight.shape[1] == weight_scale.shape[0] - ) or (weight.shape[0] // 8 != 0 or weight.shape[1] // 8 != 0): + ) or (weight.shape[0] % 8 != 0 or weight.shape[1] % 8 != 0): # Either the weight/scale is transposed or its shape is not divisible by 8. Both cases, dequantizing is the preferred way. # The transpose case is generally noticed in backward pass when we do dY@W instead of @W.T as we do for forward. # The shape case, I noticed to happen in MLP of Qwen 2.5 VL 7B where the gate proj is of shape (3420, 1280) and 3420/8=427.5 W_deq = weight_dequant(weight, weight_scale).T output = torch_matmul(x, W_deq) + output = output + bias if bias is not None else output del W_deq else: raise ValueError( diff --git a/unsloth/save.py b/unsloth/save.py index df3df13a04..472eb14933 100644 --- a/unsloth/save.py +++ b/unsloth/save.py @@ -1552,19 +1552,33 @@ def save_to_gguf( f"Error: {e}" ) print("Unsloth: Model files cleanup...") + want_full_precision = first_conversion in quantization_method if quants_created: - all_saved_locations.remove(base_gguf) - Path(base_gguf).unlink(missing_ok = True) + # convert_to_gguf may return multiple base shards plus an mmproj entry, + # so treat every initial file that is not an mmproj as part of the base set. + base_files = [ + f for f in initial_files if "-mmproj" not in os.path.basename(f).lower() + ] + if not want_full_precision: + for f in base_files: + if f in all_saved_locations: + all_saved_locations.remove(f) + Path(f).unlink(missing_ok = True) # flip the list to get [text_model, mmproj] order. for text models stays the same. all_saved_locations.reverse() + + # When the base format is preserved, move base files (incl. shards) away from + # list boundaries so example commands ([0]=model, [-1]=mmproj) stay correct. + if want_full_precision and len(all_saved_locations) > len(base_files) + 1: + for f in base_files: + if f in all_saved_locations: + all_saved_locations.remove(f) + for i, f in enumerate(base_files): + all_saved_locations.insert(1 + i, f) else: print("Unsloth: GPT-OSS model - skipping additional quantizations") - - if is_gpt_oss: want_full_precision = True - else: - want_full_precision = first_conversion in frozenset(quantization_method) print(f"Unsloth: All GGUF conversions completed successfully!") print(f"Generated files: {all_saved_locations}")