fix(save): unsloth_push_to_hub_gguf(save_method="lora") raises NameError (#7193)
* fix(save): unsloth_push_to_hub_gguf(save_method="lora") raises NameError unsloth_push_to_hub_gguf reads is_main_process at save.py:3181 but never declares it. Its twin unsloth_save_pretrained_gguf declares it (2783) and uses it the same way (2839) -- the LoRA branch was copied between the twins, the parameter it depends on was not. There is no module-level global, so the name resolves as a global load and the branch raises NameError 100% of the time. save_pretrained_gguf(save_method="lora", push_to_hub=True) raises a ValueError that tells users to "use .push_to_hub_gguf(save_method='lora') instead" -- the documented escape hatch is the broken call. Add is_main_process to the signature, positioned as in the twin, and forward it to unsloth_save_pretrained_gguf on the merged path so the parameter is not silently ignored there. Default stays True, so nothing changes for existing callers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(save): preserve GGUF push compatibility --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Etherll <61019402+Etherll@users.noreply.github.com>
This commit is contained in:
parent
1777aae37e
commit
b508c8fe89
2 changed files with 80 additions and 2 deletions
|
|
@ -8,6 +8,8 @@ regressions that pure AST checks cannot (e.g. wrong scheme/suffix/outtype passed
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import inspect
|
||||
|
||||
import pytest
|
||||
|
||||
import unsloth.save as save_mod
|
||||
|
|
@ -126,6 +128,80 @@ def test_gguf_lora_push_to_hub_is_rejected(tmp_path):
|
|||
)
|
||||
|
||||
|
||||
# The above rejection points users at push_to_hub_gguf(save_method='lora'), so that path
|
||||
# has to work; it is only ever exercised here.
|
||||
|
||||
|
||||
def test_push_to_hub_gguf_lora_dispatches(monkeypatch):
|
||||
seen = {}
|
||||
monkeypatch.setattr(
|
||||
save_mod,
|
||||
"_unsloth_save_lora_gguf",
|
||||
lambda model, tok, sd, **kw: seen.update(kw),
|
||||
)
|
||||
save_mod.unsloth_push_to_hub_gguf(
|
||||
_FakeModel(),
|
||||
"repo/id",
|
||||
tokenizer = object(),
|
||||
save_method = "lora",
|
||||
quantization_method = "q8_0",
|
||||
)
|
||||
assert seen.get("outtype") == "q8_0"
|
||||
assert seen.get("push_to_hub") is True
|
||||
|
||||
|
||||
def test_push_to_hub_gguf_lora_skips_non_main_process(monkeypatch):
|
||||
calls = []
|
||||
monkeypatch.setattr(
|
||||
save_mod,
|
||||
"_unsloth_save_lora_gguf",
|
||||
lambda *a, **kw: calls.append(kw),
|
||||
)
|
||||
result = save_mod.unsloth_push_to_hub_gguf(
|
||||
_FakeModel(),
|
||||
"repo/id",
|
||||
tokenizer = object(),
|
||||
save_method = "lora",
|
||||
is_main_process = False,
|
||||
)
|
||||
assert result is None
|
||||
assert calls == []
|
||||
|
||||
|
||||
def test_push_to_hub_gguf_skips_non_main_process_before_merged_conversion(monkeypatch):
|
||||
calls = []
|
||||
monkeypatch.setattr(
|
||||
save_mod,
|
||||
"unsloth_save_pretrained_gguf",
|
||||
lambda **kw: calls.append(kw),
|
||||
)
|
||||
result = save_mod.unsloth_push_to_hub_gguf(
|
||||
_FakeModel(),
|
||||
"repo/id",
|
||||
tokenizer = object(),
|
||||
is_main_process = False,
|
||||
)
|
||||
assert result is None
|
||||
assert calls == []
|
||||
|
||||
|
||||
def test_push_to_hub_gguf_preserves_positional_max_shard_size():
|
||||
bound = inspect.signature(save_mod.unsloth_push_to_hub_gguf).bind(
|
||||
_FakeModel(),
|
||||
"repo/id",
|
||||
object(),
|
||||
"q4_k_m",
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
"token",
|
||||
"50GB",
|
||||
)
|
||||
assert bound.arguments["max_shard_size"] == "50GB"
|
||||
assert "is_main_process" not in bound.arguments
|
||||
|
||||
|
||||
# -- torchao PTQ / QAT dispatch ------------------------------------------------------------
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -3143,6 +3143,7 @@ def unsloth_push_to_hub_gguf(
|
|||
datasets: Optional[List[str]] = None,
|
||||
save_method: str = None,
|
||||
imatrix_file = None,
|
||||
is_main_process: bool = True,
|
||||
):
|
||||
"""
|
||||
Same as .push_to_hub(...) except 4bit weights are auto
|
||||
|
|
@ -3175,11 +3176,11 @@ def unsloth_push_to_hub_gguf(
|
|||
"""
|
||||
if tokenizer is None:
|
||||
raise ValueError("Unsloth: Saving to GGUF must have a tokenizer.")
|
||||
if not is_main_process:
|
||||
return None
|
||||
|
||||
# save_method="lora" exports the adapter itself as a GGUF LoRA (not a merged model).
|
||||
if save_method is not None and str(save_method).lower() == "lora":
|
||||
if not is_main_process:
|
||||
return None # only the main rank converts and uploads, like the local lora branch
|
||||
_qm = quantization_method
|
||||
if isinstance(_qm, (list, tuple)) and len(_qm) == 1:
|
||||
_qm = _qm[0] # the gguf API allows a list; unwrap a single outtype
|
||||
|
|
@ -3233,6 +3234,7 @@ def unsloth_push_to_hub_gguf(
|
|||
first_conversion = first_conversion,
|
||||
push_to_hub = False, # Never push from here
|
||||
token = token, # forwarded so imatrix_file=True can read a gated/private upstream
|
||||
is_main_process = is_main_process,
|
||||
max_shard_size = max_shard_size,
|
||||
safe_serialization = safe_serialization,
|
||||
temporary_location = temporary_location,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue