Skip to content

Fix Mistral, Qwen#36

Merged
danielhanchen merged 154 commits into
mainfrom
nightly
Jan 20, 2025
Merged

Fix Mistral, Qwen#36
danielhanchen merged 154 commits into
mainfrom
nightly

Conversation

@danielhanchen

Copy link
Copy Markdown
Member

No description provided.

@danielhanchen danielhanchen merged commit 71229d8 into main Jan 20, 2025
danielhanchen added a commit to shimmyshimmer/unsloth-zoo-staging-2 that referenced this pull request May 5, 2026
- convert_to_gguf builds an env that prepends UNSLOTH_LLAMA_CPP_SCRIPTS_DIR/gguf-py to PYTHONPATH (when that directory exists) and passes it via env= to both subprocess.run call sites. Without this, the parent process's sys.path mods made by use_local_gguf do not propagate into the converter's child interpreter, so a user that pinned a newer convert_hf_to_gguf.py via the env var ended up running it against the default-install or system-installed gguf and saw ModuleNotFoundError or version-mismatch failures.
- use_local_gguf now also emits a logger.warning when UNSLOTH_LLAMA_CPP_SCRIPTS_DIR is set but the directory has no gguf-py/ subdirectory, mirroring the warning pattern in _resolve_local_convert_script. This surfaces the cross-version mismatch case that previously silently fell back to LLAMA_CPP_DEFAULT_DIR.
- _resolve_local_convert_script's missing-converter warning now mentions both accepted filenames (convert_hf_to_gguf.py and convert-hf-to-gguf.py) so users debugging the env var see the full set the resolver actually probes.
- Bump @lru_cache(2) on _download_convert_hf_to_gguf_cached to maxsize=8 so cross-mode toggling between local and network sources, plus a few in-place updates of a pinned converter, no longer evict cache slots and trigger redundant downloads or re-reads. Negligible memory overhead because the cached payload is small.
- Reorder check_llama_cpp's fallback list to ["convert_hf_to_gguf.py", "convert-hf-to-gguf.py"] so it matches _resolve_local_convert_script's underscore-first preference; modern llama.cpp ships only the underscore variant, so this also avoids an unnecessary stat on the legacy hyphenated name in the common case.

Blame-touched lines preserved in intent:
- The print-output `subprocess.run(command, shell=False, check=True, text=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)` continuation line (line 1443, which carries the stdout=subprocess.PIPE, stderr=subprocess.STDOUT keyword arguments) whose blame points at 70f2ce7 ("[Part 1] Complete llama.cpp Integration Overhaul ... Multi-Modal Support (unslothai#302)") still passes the same shell=False/check=True/text=True/stdout=subprocess.PIPE/stderr=subprocess.STDOUT arguments in the same order; only `env=sub_env` is added at the end so the venv-friendly invocation also picks up the pinned gguf-py via PYTHONPATH instead of the default-install gguf.
- The silent-output `subprocess.run(command, shell=False, check=True, capture_output=True)` line (line 1446) whose blame points at 6b68187 ("fix: use `sys.executable` for pip and python subprocesses to support venv environments (unslothai#503)") still passes the same command/shell=False/check=True/capture_output=True; only `env=sub_env` is added so the same sys.executable launch the original fix established also receives PYTHONPATH for the pinned gguf-py.
- The `try`/`except subprocess.CalledProcessError` block surrounding both calls (whose blame points at 70f2ce7 "[Part 1] Complete llama.cpp Integration Overhaul ... Multi-Modal Support (unslothai#302)" and 71229d8 "Fix Mistral, Qwen (unslothai#36)") is unchanged: same RuntimeError type, same command-string formatting, same stdout-print-on-failure path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants