[staging CI] unslothai/unsloth#4222 - #131
Closed
danielhanchen wants to merge 9 commits into
Closed
danielhanchen wants to merge 9 commits into
danielhanchen wants to merge 9 commits into
Conversation
- Fix #3544: Add revision parameter to AutoConfig, AutoModelForCausalLM, AutoModelForSequenceClassification, and load_correct_tokenizer calls in FastLlamaModel.from_pretrained. This enables loading specific model revisions/branches from HuggingFace Hub. - Fix #3667: Escape single quotes in system messages before substituting into Jinja2 templates. This prevents TemplateSyntaxError when system messages contain apostrophes (e.g., "user's" in Vicuna templates). Signed-off-by: majiayu000 <1835304752@qq.com> (cherry picked from commit b0a6e4154b1bca9ed9bc06bdd1a83da1007dd6bf)
- Add revision to load_vllm_kwargs in llama.py to fix config/weights mismatch - Add revision to PEFT AutoConfig calls in loader.py (FastLanguageModel & FastModel) Addresses reviewer feedback from @chatgpt-codex-connector and @Datta0 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> (cherry picked from commit 14f89e453173f3279c885ed37d5f2a9cf10793df)
Propagate revision parameter to all from_pretrained calls in vision.py to ensure consistent version pinning for vision models. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> (cherry picked from commit c5aa4ec92783345eaa0286bd876386cb2767d990)
The branch has drifted nine months. Resolving the merge to main's content
everywhere, because every hunk it carried is now either landed, superseded
or actively harmful:
- chat_templates.py escaping: superseded. main no longer uses re.sub, and
_escape_jinja_literal already covers backslash, both quotes and CR.
- loader.py AutoConfig(revision = ...): already on main.
- vision.py adding revision to the FastBaseModel.from_pretrained signature:
harmful. revision arrives via **kwargs there, and binding it as a named
parameter drops it from the weight load and from kwargs.get("revision").
- llama.py load_vllm(revision = ...): raises TypeError. load_vllm has no
revision parameter and load_vllm_kwargs is not filtered.
The revision fix itself follows in the next commit.
FastLlamaModel.from_pretrained took a `revision` argument and never read it, so the config, the weights and the tokenizer all came from the repo's default branch while the caller believed they had pinned a ref. Reported in #3544 by someone versioning their fine-tunes with branches, which makes it a silently wrong base checkpoint rather than an error. Forward it in llama.py (both AutoConfig loads, the three model loads, the tokenizer, the prefetch warm and the fp8 scale restore), plumb it through load_correct_tokenizer, and read it from kwargs in vision.py for the four AutoConfig, two processor and two tokenizer loads plus the VLM processor fallback. vision.py must not bind it as a named parameter: the weight load there forwards **kwargs, so binding it would drop it from that load. model_name is not always the repo the caller named. get_model_name can swap in a pre-quantized mirror, _offline_quantize_to_fp8 an fp8 temp dir, ModelScope a local snapshot, and fast_inference_setup a -bnb-4bit variant, and use_exact_model_name only gates the first of those. A ref from the original repo does not exist on the substitute, so _revision_for_resolved_repo drops it with a warning naming both repos when the resolution changed the name. The adapter load keeps the caller's revision, since that one really is for old_model_name. Supersedes the earlier attempt on this branch, whose chat-template hunk is handled by #7731 and #7746, whose vision.py signature change caused the drop described above, and whose load_vllm(revision = ...) raised TypeError because load_vllm has no such parameter. Fixes #3544
for more information, see https://pre-commit.ci
Four fixes from review: - The gate ran after the AutoConfig and PeftConfig probes, which already used the raw revision against the resolved name, so a pinned load_in_4bit load failed against the mirror instead of warning. Gate right after the resolution block and point both probes at the gated value, then re-gate before dispatch for the later fast_inference_setup remap. Feeding the second call the first result keeps the warning to one. - On a PEFT load model_name is necessarily the base model, so the late gate warned "Ignoring revision" for every versioned adapter and told the caller to pass use_exact_model_name, which cannot stop an adapter resolving its base. Skip the late gate for PEFT; PeftModel.from_pretrained already loads the adapter with the caller's revision. - load_vllm takes no revision, so vLLM fetches the default branch. Pinning only the config and the tokenizer put two refs in one model, which is worse than the old behaviour of ignoring the revision outright. Drop the pin with a warning before the config load whenever vLLM owns the weights. - _hub_repo_or_local_path resolved a cached snapshot without the revision, so an offline or local_files_only tokenizer load silently got the default ref: a revision handed to from_pretrained cannot re-point a local directory. Thread it into _resolve_hub_repo_local_dir and both call sites. Five new tests, one per fix, all failing before it.
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
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
2 participants
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.
Disposable CI run for unslothai/unsloth#4222. Do not merge; closed after CI.