Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions unsloth_zoo/patching_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,12 @@ def patch_compiling_bitsandbytes():
# All Unsloth Zoo code licensed under LGPLv3
os.environ["UNSLOTH_PATCHED"] = "1"

import bitsandbytes
try:
import bitsandbytes
except Exception:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

It's a best practice to catch specific exceptions rather than the general Exception. In this case, an ImportError is expected if bitsandbytes is not installed. Using except ImportError: is more precise and prevents masking other potential errors during the import process.

Suggested change
except Exception:
except ImportError:

# bitsandbytes is optional (eg, AMD ROCm environments). If it's not
# installed, just skip this patch.
return
if Version(bitsandbytes.__version__) >= Version("0.46.0"):
if os.environ.get("UNSLOTH_ENABLE_LOGGING", "0") == "1":
print("Unsloth: Bitsandbytes >= 0.46.0 supports torch.compile - enabling.")
Expand Down Expand Up @@ -307,12 +312,13 @@ def patch_model_and_tokenizer(
# BnB default dtype seems to be float16!
try:
from bitsandbytes.nn import Linear4bit as Bnb_Linear4bit
except:
raise ImportError("Unsloth: Please install bitsandbytes via `pip install bitsandbytes`")
except Exception:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To make the code more robust, it's better to catch a more specific exception. Since you're handling a potentially missing optional dependency, ImportError is the more appropriate exception to catch here instead of the broad Exception.

Suggested change
except Exception:
except ImportError:

Bnb_Linear4bit = None
try:
from peft.tuners.lora import Linear4bit as Peft_Linear4bit
except:
raise ImportError("Unsloth: Please install peft via `pip install peft`")
except Exception:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to the previous comment, catching a specific ImportError is preferred over a general Exception. This ensures that you are only handling the case where the module is not found and not suppressing other unexpected errors.

Suggested change
except Exception:
except ImportError:

Peft_Linear4bit = None
_bnb_types = tuple(t for t in (Bnb_Linear4bit, Peft_Linear4bit) if t is not None)
pass

# Get most likely the correct data-type of the model
Expand Down Expand Up @@ -398,7 +404,7 @@ def __fix_dtype(config):

# Check all params and patch!
for name, module in model.named_modules():
if isinstance(module, (Bnb_Linear4bit, Peft_Linear4bit)):
if _bnb_types and isinstance(module, _bnb_types):
weight = module.weight
# Check if quant_state exists for vision models like unsloth/Llama-3.2-11B-Vision-Instruct-bnb-4bit, unsloth/granite-vision-3.2-2b
if not hasattr(weight, 'quant_state'):
Expand Down
10 changes: 8 additions & 2 deletions unsloth_zoo/peft_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,17 @@ def get_lora_layer_modules():
for file in files:
if file == "__init__.py" or not file.endswith(".py"): continue
item = f"peft.tuners.lora.{file[:-len('.py')]}"
exec(f"import {item}", locals(), globals())
try:
exec(f"import {item}", locals(), globals())
except Exception:
continue
Comment on lines +149 to +152

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid silently skipping LoRA modules on unexpected import errors

Catching all exceptions here means any failure inside a LoRA module (e.g., a real bug or a missing dependency unrelated to bitsandbytes) is silently ignored, so its Linear* classes are never added to Linear_LoRA_Layers. In that scenario, models using that module type won’t be patched and LoRA layers may be missed without any warning, leading to incomplete training behavior that is hard to diagnose. If the intent is only to ignore optional backends, consider narrowing the exception to the specific missing dependency or at least logging unexpected import failures.

Useful? React with 👍 / 👎.

modules = dir(eval(item))
modules = [x for x in modules if x.startswith("Linear") or x.endswith("Linear")]
if len(modules) == 0: continue
exec(f"from {item} import ({', '.join(modules)})", locals(), globals())
try:
exec(f"from {item} import ({', '.join(modules)})", locals(), globals())
except Exception:
continue
Linear_LoRA_Layers += [(eval(x), item, x,) for x in modules]
Comment on lines +149 to 160
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using exec() and eval() for dynamic imports can introduce security vulnerabilities and make the code harder to debug and maintain. A safer and cleaner approach is to use the importlib module. This refactoring will make the code more robust and readable.

Please also add import importlib at the top of the file.

Suggested change
try:
exec(f"import {item}", locals(), globals())
except Exception:
continue
modules = dir(eval(item))
modules = [x for x in modules if x.startswith("Linear") or x.endswith("Linear")]
if len(modules) == 0: continue
exec(f"from {item} import ({', '.join(modules)})", locals(), globals())
try:
exec(f"from {item} import ({', '.join(modules)})", locals(), globals())
except Exception:
continue
Linear_LoRA_Layers += [(eval(x), item, x,) for x in modules]
try:
module_obj = importlib.import_module(item)
except ImportError:
# This can happen if bitsandbytes is not installed for modules like bnb.py
continue
module_names = [
x for x in dir(module_obj)
if (x.startswith("Linear") or x.endswith("Linear")) and isinstance(getattr(module_obj, x, None), type)
]
for module_name in module_names:
layer_class = getattr(module_obj, module_name)
Linear_LoRA_Layers.append((layer_class, item, module_name))

pass
return tuple(Linear_LoRA_Layers)
Expand Down
7 changes: 5 additions & 2 deletions unsloth_zoo/saving_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@
"""

import torch
import bitsandbytes as bnb
try:
import bitsandbytes as bnb
except Exception:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

It's better to catch specific exceptions. Here, ImportError is the expected exception if bitsandbytes is not installed. Using except ImportError: is safer as it won't hide other unexpected errors that might occur during the import.

Suggested change
except Exception:
except ImportError:

bnb = None
try:
from huggingface_hub import get_token
except:
Expand All @@ -84,7 +87,7 @@ def find_skipped_quantized_modules(model):
skipped_modules = []
quantized_modules = []
for name, module in model.named_modules():
if isinstance(module, bnb.nn.Linear4bit):
if (bnb is not None) and isinstance(module, bnb.nn.Linear4bit):
if hasattr(module.weight, 'quant_state') and module.weight.quant_state is not None:
quantized_modules.append(name)
else:
Expand Down