Respect requested 4bit/8bit for local '-bf16' directories - #6726
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the model loader in unsloth/models/loader.py to prevent disabling requested 4-bit, 8-bit, or FP8 quantization when loading from a local directory whose name ends with -bf16. The review feedback correctly identifies that os.path.isdir does not automatically expand tilde (~) paths, which would cause local directories using this shorthand to be incorrectly treated as remote hub repositories. It is recommended to wrap the path check with os.path.expanduser to ensure proper directory resolution.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| # Change -BF16 to all False for 4bit, 8bit etc | ||
| if model_name.lower().endswith("-bf16"): | ||
| # '-bf16' hub repos load bf16; for a local dir keep the requested 4bit/8bit | ||
| if model_name.lower().endswith("-bf16") and not os.path.isdir(model_name): |
There was a problem hiding this comment.
If the user specifies a local path using the tilde (~) expansion (e.g., ~/models/my-model-bf16), os.path.isdir will return False because it does not automatically expand ~. This will cause the loader to mistakenly treat it as a hub repo and force-disable the requested 4bit/8bit quantization.
Using os.path.expanduser(model_name) resolves this issue by correctly expanding the user directory before checking if it is a directory.
| if model_name.lower().endswith("-bf16") and not os.path.isdir(model_name): | |
| if model_name.lower().endswith("-bf16") and not os.path.isdir(os.path.expanduser(model_name)): |
References
- When loading files from a local directory path, check if the path is a local directory first to avoid falling back to Hugging Face Hub download functions which would treat the local path as a repository ID.
| # Change -BF16 to all False for 4bit, 8bit etc | ||
| if model_name.lower().endswith("-bf16"): | ||
| # '-bf16' hub repos load bf16; for a local dir keep the requested 4bit/8bit | ||
| if model_name.lower().endswith("-bf16") and not os.path.isdir(model_name): |
There was a problem hiding this comment.
If the user specifies a local path using the tilde (~) expansion (e.g., ~/models/my-model-bf16), os.path.isdir will return False because it does not automatically expand ~. This will cause the loader to mistakenly treat it as a hub repo and force-disable the requested 4bit/8bit quantization.
Using os.path.expanduser(model_name) resolves this issue by correctly expanding the user directory before checking if it is a directory.
| if model_name.lower().endswith("-bf16") and not os.path.isdir(model_name): | |
| if model_name.lower().endswith("-bf16") and not os.path.isdir(os.path.expanduser(model_name)): |
References
- When loading files from a local directory path, check if the path is a local directory first to avoid falling back to Hugging Face Hub download functions which would treat the local path as a repository ID.
| # Change -BF16 to all False for 4bit, 8bit etc | ||
| if model_name.lower().endswith("-bf16"): | ||
| # '-bf16' hub repos load bf16; for a local dir keep the requested 4bit/8bit | ||
| if model_name.lower().endswith("-bf16") and not os.path.isdir(model_name): |
There was a problem hiding this comment.
If the user specifies a local path using the tilde (~) expansion (e.g., ~/models/my-model-bf16), os.path.isdir will return False because it does not automatically expand ~. This will cause the loader to mistakenly treat it as a hub repo and force-disable the requested 4bit/8bit quantization.
Using os.path.expanduser(model_name) resolves this issue by correctly expanding the user directory before checking if it is a directory.
| if model_name.lower().endswith("-bf16") and not os.path.isdir(model_name): | |
| if model_name.lower().endswith("-bf16") and not os.path.isdir(os.path.expanduser(model_name)): |
References
- When loading files from a local directory path, check if the path is a local directory first to avoid falling back to Hugging Face Hub download functions which would treat the local path as a repository ID.
| # Change -BF16 to all False for 4bit, 8bit etc | ||
| if model_name.lower().endswith("-bf16"): | ||
| # '-bf16' hub repos load bf16; for a local dir keep the requested 4bit/8bit | ||
| if model_name.lower().endswith("-bf16") and not os.path.isdir(model_name): |
There was a problem hiding this comment.
If the user specifies a local path using the tilde (~) expansion (e.g., ~/models/my-model-bf16), os.path.isdir will return False because it does not automatically expand ~. This will cause the loader to mistakenly treat it as a hub repo and force-disable the requested 4bit/8bit quantization.
Using os.path.expanduser(model_name) resolves this issue by correctly expanding the user directory before checking if it is a directory.
| if model_name.lower().endswith("-bf16") and not os.path.isdir(model_name): | |
| if model_name.lower().endswith("-bf16") and not os.path.isdir(os.path.expanduser(model_name)): |
References
- When loading files from a local directory path, check if the path is a local directory first to avoid falling back to Hugging Face Hub download functions which would treat the local path as a repository ID.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b851e50ee6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if model_name.lower().endswith("-bf16") and not os.path.isdir( | ||
| os.path.expanduser(model_name) | ||
| ): |
There was a problem hiding this comment.
Preserve bf16 override for local -bf16 loads
When a caller follows the new local-dir guidance and sets load_in_16bit=True without also overriding the default load_in_4bit=True, this branch now skips clearing load_in_4bit because the path exists. In the FastLanguageModel path, load_in_16bit is not validated or forwarded to the architecture loader later (dispatch_model.from_pretrained only receives load_in_4bit), so a local *-bf16 directory still loads as 4-bit instead of the requested bf16; before this change the suffix block cleared load_in_4bit for these paths.
Useful? React with 👍 / 👎.
A model path ending in -bf16 unconditionally forced 16-bit loading, so a LOCAL checkpoint directory whose name happens to end in -bf16 could never be loaded in 4-bit, 8-bit or fp8: the suffix rule silently overrode the caller's quantization flags. Hub repo ids keep the existing behavior (the suffix is a publishing convention there), but for a local directory (expanduser-aware, so tilde paths are detected too) the requested quantization is preserved unless the caller explicitly passes load_in_16bit=True.
503b95f to
66b8c9a
Compare
|
Rebased onto current main and applied the same condition to all four -bf16 sites (two more were added on main since this branch was cut). Semantics unchanged: hub repo ids ending in -bf16 still force 16-bit, a local directory keeps the requested quantization unless load_in_16bit=True is passed, and tilde paths are detected via expanduser. |
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the model loading logic in unsloth/models/loader.py to ensure that local directories ending with -bf16 retain their requested quantization settings unless load_in_16bit is explicitly set, while hub repositories ending in -bf16 continue to load in bf16. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
The |
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Problem
The loader treats any
model_nameending in-bf16as a request to load bf16 and force-disables 4bit/8bit. That suffix is our hub naming convention for bf16 weight repos, but it also fires for a local directory that just happens to be named...-bf16. Sinceload_in_4bitdefaults toTrue, pointing at a local bf16 checkpoint dir named-bf16silently loads it in bf16 and uses roughly 4x the memory the user asked for.Fix
Only apply the
-bf16suffix override for hub names, not local directories:-bf16are unchanged (the convention still loads bf16).-bf16directory now honors the requested quantization (load_in_4bit/load_in_8bit). To force bf16 from a local dir, passload_in_16bit=True.Applied to all four
from_pretrainedpaths inloader.py.os.path.isdiris cross platform.