Refactor Loraconfig and model constructor #3123
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughAdds a builder-based adapter loading framework with a factory, abstract base, and LoRA implementation. Integrates builder path into load_adapter and model loader, with config-only support and fallbacks to legacy logic. Updates package exports for adapters/builders and adjusts signatures to pass inference and config_only flags. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (10)
src/axolotl/loaders/adapters/__init__.py (1)
9-13: Sort all to satisfy Ruff RUF022.Alphabetize for consistency.
Apply:
__all__ = [ - "AdapterBuilderFactory", - "BaseAdapterBuilder", - "LoraAdapterBuilder", + "AdapterBuilderFactory", + "BaseAdapterBuilder", + "LoraAdapterBuilder", ]src/axolotl/loaders/adapters/builders/__init__.py (1)
7-11: Sort all to satisfy Ruff RUF022.Alphabetize for consistency.
Apply:
__all__ = [ - "BaseAdapterBuilder", - "AdapterBuilderFactory", - "LoraAdapterBuilder", + "AdapterBuilderFactory", + "BaseAdapterBuilder", + "LoraAdapterBuilder", ]src/axolotl/loaders/adapter.py (2)
191-209: Broadened fallback is fine, but tighten logging and remove redundancy.
- Keep broad catch if you want universal fallback, but include traceback.
- The adapter None check is redundant after the early return.
Apply:
- except Exception as e: - LOG.debug( - f"Builder pattern failed, falling back to legacy adapter loading: {e}" - ) + except Exception as e: + LOG.debug( + f"Builder pattern failed, falling back to legacy adapter loading: {e}", + exc_info=True, + ) @@ - if adapter is None: - return model, None
185-187: Gate enable_input_require_grads by training mode.Avoid extra memory in pure inference flows.
Apply:
- if hasattr(model, "enable_input_require_grads"): + if not inference and hasattr(model, "enable_input_require_grads"): model.enable_input_require_grads()Please confirm no training features rely on this during inference in your codepaths.
src/axolotl/loaders/adapters/builders/factory.py (4)
13-16: Annotate mutable class attribute with ClassVar to satisfy Ruff RUF012.Prevents treating
_buildersas an instance attribute and quiets linters.-from typing import Dict, Type +from typing import Dict, Type, ClassVar @@ - _builders: Dict[str, Type[BaseAdapterBuilder]] = { + _builders: ClassVar[Dict[str, Type[BaseAdapterBuilder]]] = { "lora": LoraAdapterBuilder, "qlora": LoraAdapterBuilder, }
18-33: Normalize keys and validate registrations in register_builder.Avoid case/whitespace mismatches and fail fast if a non-builder is registered.
def register_builder( cls, adapter_type: str, builder_class: Type[BaseAdapterBuilder] ): @@ - cls._builders[adapter_type] = builder_class + adapter_type = adapter_type.strip().lower() + if not issubclass(builder_class, BaseAdapterBuilder): + raise TypeError( + f"builder_class must subclass BaseAdapterBuilder, got {builder_class!r}" + ) + cls._builders[adapter_type] = builder_class LOG.info( f"Registered adapter builder for '{adapter_type}': {builder_class.__name__}" )
35-58: Lowercase inputs and tighten error message; reduce TRY003 noise.Prevents subtle lookup bugs and trims the raised message.
- def create_builder(cls, adapter_type: str, cfg: DictDefault) -> BaseAdapterBuilder: + def create_builder(cls, adapter_type: str, cfg: DictDefault) -> BaseAdapterBuilder: @@ - if adapter_type not in cls._builders: - available_types = list(cls._builders.keys()) - raise ValueError( - f"Unsupported adapter type: {adapter_type}. " - f"Available types: {available_types}" - ) + adapter_type = (adapter_type or "").strip().lower() + if adapter_type not in cls._builders: + available_types = sorted(cls._builders.keys()) + raise ValueError( + f"Unsupported adapter type {adapter_type!r}. Supported: {', '.join(available_types)}" + )
61-68: Return a stable, sorted list of supported adapters.Predictable ordering helps logs/tests.
- return list(cls._builders.keys()) + return sorted(cls._builders.keys())src/axolotl/loaders/adapters/builders/base.py (2)
73-79: Use f-string conversion flag instead of repr() (RUF010).- LOG.info(f"found linear modules: {repr(sorted(linear_names))}") + LOG.info(f"found linear modules: {sorted(linear_names)!r}")
20-36: Base class mixes LoRA-specific config into an abstract method.Not blocking, but consider keeping
build_configpurely abstract and moving LoRA-only assembly toLoraAdapterBuilderto avoid coupling as more adapters are added.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/axolotl/loaders/adapter.py(2 hunks)src/axolotl/loaders/adapters/__init__.py(1 hunks)src/axolotl/loaders/adapters/builders/__init__.py(1 hunks)src/axolotl/loaders/adapters/builders/base.py(1 hunks)src/axolotl/loaders/adapters/builders/factory.py(1 hunks)src/axolotl/loaders/adapters/builders/lora.py(1 hunks)src/axolotl/loaders/model.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
src/axolotl/loaders/adapters/builders/__init__.py (3)
src/axolotl/loaders/adapters/builders/base.py (1)
BaseAdapterBuilder(13-249)src/axolotl/loaders/adapters/builders/factory.py (1)
AdapterBuilderFactory(10-68)src/axolotl/loaders/adapters/builders/lora.py (1)
LoraAdapterBuilder(10-71)
src/axolotl/loaders/adapters/__init__.py (3)
src/axolotl/loaders/adapters/builders/factory.py (1)
AdapterBuilderFactory(10-68)src/axolotl/loaders/adapters/builders/base.py (1)
BaseAdapterBuilder(13-249)src/axolotl/loaders/adapters/builders/lora.py (1)
LoraAdapterBuilder(10-71)
src/axolotl/loaders/adapters/builders/lora.py (3)
src/axolotl/utils/schemas/peft.py (1)
LoraConfig(28-185)src/axolotl/utils/logging.py (1)
get_logger(42-49)src/axolotl/loaders/adapters/builders/base.py (9)
BaseAdapterBuilder(13-249)prepare_target_modules(53-91)prepare_target_parameters(93-111)build_common_config_kwargs(113-151)setup_quantization_for_training(153-168)load_pretrained_adapter(189-218)create_peft_model(220-233)print_trainable_parameters(235-249)setup_quantization_for_training_post_build(170-187)
src/axolotl/loaders/adapter.py (3)
src/axolotl/loaders/adapters/builders/factory.py (2)
AdapterBuilderFactory(10-68)create_builder(35-58)src/axolotl/loaders/adapters/builders/lora.py (2)
build_config(13-47)build_model(49-71)src/axolotl/loaders/adapters/builders/base.py (2)
build_config(21-36)build_model(39-51)
src/axolotl/loaders/adapters/builders/base.py (4)
src/axolotl/utils/schemas/peft.py (2)
PeftConfig(17-25)LoftQConfig(8-14)src/axolotl/utils/dict.py (1)
DictDefault(6-38)src/axolotl/utils/logging.py (1)
get_logger(42-49)src/axolotl/loaders/adapter.py (3)
find_all_linear_names(49-66)setup_quantized_meta_for_peft(29-38)setup_quantized_peft_meta_for_training(41-46)
src/axolotl/loaders/adapters/builders/factory.py (4)
src/axolotl/utils/dict.py (1)
DictDefault(6-38)src/axolotl/utils/logging.py (1)
get_logger(42-49)src/axolotl/loaders/adapters/builders/base.py (1)
BaseAdapterBuilder(13-249)src/axolotl/loaders/adapters/builders/lora.py (1)
LoraAdapterBuilder(10-71)
src/axolotl/loaders/model.py (3)
src/axolotl/loaders/adapters/builders/factory.py (2)
AdapterBuilderFactory(10-68)create_builder(35-58)src/axolotl/loaders/adapters/builders/lora.py (1)
build_config(13-47)src/axolotl/loaders/adapter.py (2)
load_lora(69-161)load_adapter(164-209)
🪛 Ruff (0.12.2)
src/axolotl/loaders/adapters/builders/__init__.py
7-11: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
src/axolotl/loaders/adapter.py
189-189: Consider moving this statement to an else block
(TRY300)
191-191: Do not catch blind exception: Exception
(BLE001)
src/axolotl/loaders/adapters/builders/base.py
77-77: Use explicit conversion flag
Replace with conversion flag
(RUF010)
src/axolotl/loaders/adapters/builders/factory.py
13-16: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
51-54: Avoid specifying long messages outside the exception class
(TRY003)
src/axolotl/loaders/model.py
384-384: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: PyTest from Source Dist (3.11, 2.7.1)
- GitHub Check: PyTest from Source Dist (3.11, 2.8.0)
- GitHub Check: PyTest (3.11, 2.8.0)
- GitHub Check: PyTest from Source Dist (3.11, 2.6.0)
- GitHub Check: PyTest (3.11, 2.6.0)
- GitHub Check: PyTest (3.11, 2.7.1)
🔇 Additional comments (6)
src/axolotl/loaders/adapters/__init__.py (1)
3-7: Public re-exports look good.Clear, minimal surface for the adapters package.
src/axolotl/loaders/adapters/builders/__init__.py (1)
3-6: Good aggregation of builder APIs.Keeps imports ergonomic for callers.
src/axolotl/loaders/adapter.py (2)
24-24: Import location OK.No cycle risk given the lazy imports inside builder base methods.
200-207: Ensureconfig_onlyconsistency: bothload_loraandload_llama_adaptershould use a return type ofTuple[ModelType, Optional[ConfigType]], allowingNonewhenconfig_only=True.src/axolotl/loaders/model.py (1)
393-394: Good: pass inference through to load_adapter.This keeps the adapter path inference-aware. No change needed.
src/axolotl/loaders/adapters/builders/lora.py (1)
31-47: LoRA config construction looks correct.Fields align with cfg; target modules/params and common kwargs merged properly.
src/axolotl/loaders/adapter.py
Outdated
| config_only: bool = False, | ||
| ) -> tuple[PreTrainedModel | PeftModel | PeftMixedModel, PeftConfig | None]: |
There was a problem hiding this comment.
Return type must include None for config_only=True.
Function can return (None, config) but the annotation excludes None for the model.
Apply:
def load_adapter(
@@
- config_only: bool = False,
-) -> tuple[PreTrainedModel | PeftModel | PeftMixedModel, PeftConfig | None]:
+ config_only: bool = False,
+) -> tuple[PreTrainedModel | PeftModel | PeftMixedModel | None, PeftConfig | None]:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| config_only: bool = False, | |
| ) -> tuple[PreTrainedModel | PeftModel | PeftMixedModel, PeftConfig | None]: | |
| config_only: bool = False, | |
| ) -> tuple[PreTrainedModel | PeftModel | PeftMixedModel | None, PeftConfig | None]: |
🤖 Prompt for AI Agents
In src/axolotl/loaders/adapter.py around lines 169-170, the return type
annotation disallows returning None for the model though the function can return
(None, config) when config_only=True; update the annotation to allow None for
the model (e.g. tuple[PreTrainedModel | PeftModel | PeftMixedModel | None,
PeftConfig | None]) and audit callers to handle the possible None model
accordingly.
| try: | ||
| if adapter is None: | ||
| return model, None | ||
| builder = AdapterBuilderFactory.create_builder(adapter, cfg) | ||
|
|
||
| if not builder: | ||
| LOG.warning(f"No builder found for adapter type '{adapter}'") | ||
| return model, None | ||
|
|
||
| config = builder.build_config(model) | ||
|
|
||
| if config_only: | ||
| return None, config | ||
|
|
||
| if hasattr(model, "enable_input_require_grads"): | ||
| model.enable_input_require_grads() | ||
|
|
||
| model = builder.build_model(model, config) | ||
| return model, config | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Propagate inference to builder and remove dead check.
- inference is not forwarded; pretrained adapters will be loaded as trainable during inference.
- AdapterBuilderFactory.create_builder never returns None, so the "if not builder" branch is unreachable.
Apply:
- builder = AdapterBuilderFactory.create_builder(adapter, cfg)
-
- if not builder:
- LOG.warning(f"No builder found for adapter type '{adapter}'")
- return model, None
+ builder = AdapterBuilderFactory.create_builder(adapter, cfg)
@@
- model = builder.build_model(model, config)
+ model = builder.build_model(model, config, inference=inference)
return model, configAnd update builder signatures accordingly (outside this file):
--- a/src/axolotl/loaders/adapters/builders/base.py
+++ b/src/axolotl/loaders/adapters/builders/base.py
@@
- def build_model(self, model: PreTrainedModel, config: PeftConfig) -> PeftModel:
+ def build_model(
+ self, model: PreTrainedModel, config: PeftConfig, *, inference: bool = False
+ ) -> PeftModel:
@@
- if self.cfg.lora_model_dir:
- model = self.load_pretrained_adapter(model)
+ if self.cfg.lora_model_dir:
+ model = self.load_pretrained_adapter(model, inference=inference)
else:
model = self.create_peft_model(model, config)--- a/src/axolotl/loaders/adapters/builders/lora.py
+++ b/src/axolotl/loaders/adapters/builders/lora.py
@@
- def build_model(self, model: PreTrainedModel, config: LoraConfig) -> PeftModel:
+ def build_model(
+ self, model: PreTrainedModel, config: LoraConfig, *, inference: bool = False
+ ) -> PeftModel:
@@
- if self.cfg.lora_model_dir:
+ if self.cfg.lora_model_dir:
LOG.debug("Loading pretrained PEFT - LoRA")
- model = self.load_pretrained_adapter(model)
+ model = self.load_pretrained_adapter(model, inference=inference)
else:
model = self.create_peft_model(model, config)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try: | |
| if adapter is None: | |
| return model, None | |
| builder = AdapterBuilderFactory.create_builder(adapter, cfg) | |
| if not builder: | |
| LOG.warning(f"No builder found for adapter type '{adapter}'") | |
| return model, None | |
| config = builder.build_config(model) | |
| if config_only: | |
| return None, config | |
| if hasattr(model, "enable_input_require_grads"): | |
| model.enable_input_require_grads() | |
| model = builder.build_model(model, config) | |
| return model, config | |
| try: | |
| if adapter is None: | |
| return model, None | |
| builder = AdapterBuilderFactory.create_builder(adapter, cfg) | |
| config = builder.build_config(model) | |
| if config_only: | |
| return None, config | |
| if hasattr(model, "enable_input_require_grads"): | |
| model.enable_input_require_grads() | |
| model = builder.build_model(model, config, inference=inference) | |
| return model, config |
🧰 Tools
🪛 Ruff (0.12.2)
189-189: Consider moving this statement to an else block
(TRY300)
| @abstractmethod | ||
| def build_model(self, model: PreTrainedModel, config: PeftConfig) -> PeftModel: | ||
| """Build the PEFT model""" | ||
| self.setup_quantization_for_training(model) | ||
|
|
||
| if self.cfg.lora_model_dir: | ||
| model = self.load_pretrained_adapter(model) | ||
| else: | ||
| model = self.create_peft_model(model, config) | ||
|
|
||
| self.print_trainable_parameters(model) | ||
| self.setup_quantization_for_training_post_build(model) | ||
|
|
||
| return model |
There was a problem hiding this comment.
Propagate inference through the base builder.
Ensures consistent behavior across all builders when loading pretrained adapters.
- def build_model(self, model: PreTrainedModel, config: PeftConfig) -> PeftModel:
+ def build_model(
+ self, model: PreTrainedModel, config: PeftConfig, *, inference: bool = False
+ ) -> PeftModel:
"""Build the PEFT model"""
self.setup_quantization_for_training(model)
if self.cfg.lora_model_dir:
- model = self.load_pretrained_adapter(model)
+ model = self.load_pretrained_adapter(model, inference=inference)
else:
model = self.create_peft_model(model, config)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @abstractmethod | |
| def build_model(self, model: PreTrainedModel, config: PeftConfig) -> PeftModel: | |
| """Build the PEFT model""" | |
| self.setup_quantization_for_training(model) | |
| if self.cfg.lora_model_dir: | |
| model = self.load_pretrained_adapter(model) | |
| else: | |
| model = self.create_peft_model(model, config) | |
| self.print_trainable_parameters(model) | |
| self.setup_quantization_for_training_post_build(model) | |
| return model | |
| @abstractmethod | |
| def build_model( | |
| self, model: PreTrainedModel, config: PeftConfig, *, inference: bool = False | |
| ) -> PeftModel: | |
| """Build the PEFT model""" | |
| self.setup_quantization_for_training(model) | |
| if self.cfg.lora_model_dir: | |
| model = self.load_pretrained_adapter(model, inference=inference) | |
| else: | |
| model = self.create_peft_model(model, config) | |
| self.print_trainable_parameters(model) | |
| self.setup_quantization_for_training_post_build(model) | |
| return model |
🤖 Prompt for AI Agents
In src/axolotl/loaders/adapters/builders/base.py around lines 38-51, the base
build_model currently branches to load a pretrained adapter without propagating
the builder's inference/training mode or relevant flags; update build_model so
that when self.cfg.lora_model_dir is truthy you call load_pretrained_adapter in
a way that preserves the builder's inference mode and any quantization/training
flags (e.g., pass an inference/training argument or the builder's context), and
if load_pretrained_adapter signature needs it, add a parameter to accept and
forward that flag; ensure both code paths (load_pretrained_adapter and
create_peft_model) receive the same inference/training/quantization context,
then keep the existing setup_quantization_for_training_post_build and
print_trainable_parameters calls and return the model.
| def build_model(self, model: PreTrainedModel, config: LoraConfig) -> PeftModel: | ||
| """ | ||
| Build LoRA model. | ||
| Args: | ||
| model: Base model | ||
| config: LoRA configuration | ||
| Returns: | ||
| PeftModel: Model with LoRA adapter applied | ||
| """ | ||
| self.setup_quantization_for_training(model) | ||
|
|
||
| if self.cfg.lora_model_dir: | ||
| LOG.debug("Loading pretrained PEFT - LoRA") | ||
| model = self.load_pretrained_adapter(model) | ||
| else: | ||
| model = self.create_peft_model(model, config) | ||
|
|
||
| self.print_trainable_parameters(model) | ||
| self.setup_quantization_for_training_post_build(model) | ||
|
|
||
| return model |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Respect inference mode when loading pretrained adapters.
build_model ignores inference, so PeftModel.from_pretrained(..., is_trainable=(not inference)) always treats it as trainable. Thread inference through to load_pretrained_adapter.
- def build_model(self, model: PreTrainedModel, config: LoraConfig) -> PeftModel:
+ def build_model(
+ self, model: PreTrainedModel, config: LoraConfig, *, inference: bool = False
+ ) -> PeftModel:
@@
- if self.cfg.lora_model_dir:
+ if self.cfg.lora_model_dir:
LOG.debug("Loading pretrained PEFT - LoRA")
- model = self.load_pretrained_adapter(model)
+ model = self.load_pretrained_adapter(model, inference=inference)
else:
model = self.create_peft_model(model, config)Follow-up (outside this file): update the call site to pass inference (see suggested change to load_adapter below).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def build_model(self, model: PreTrainedModel, config: LoraConfig) -> PeftModel: | |
| """ | |
| Build LoRA model. | |
| Args: | |
| model: Base model | |
| config: LoRA configuration | |
| Returns: | |
| PeftModel: Model with LoRA adapter applied | |
| """ | |
| self.setup_quantization_for_training(model) | |
| if self.cfg.lora_model_dir: | |
| LOG.debug("Loading pretrained PEFT - LoRA") | |
| model = self.load_pretrained_adapter(model) | |
| else: | |
| model = self.create_peft_model(model, config) | |
| self.print_trainable_parameters(model) | |
| self.setup_quantization_for_training_post_build(model) | |
| return model | |
| def build_model( | |
| self, model: PreTrainedModel, config: LoraConfig, *, inference: bool = False | |
| ) -> PeftModel: | |
| """ | |
| Build LoRA model. | |
| Args: | |
| model: Base model | |
| config: LoRA configuration | |
| inference: Whether to build in inference mode (non-trainable) | |
| Returns: | |
| PeftModel: Model with LoRA adapter applied | |
| """ | |
| self.setup_quantization_for_training(model) | |
| if self.cfg.lora_model_dir: | |
| LOG.debug("Loading pretrained PEFT - LoRA") | |
| model = self.load_pretrained_adapter(model, inference=inference) | |
| else: | |
| model = self.create_peft_model(model, config) | |
| self.print_trainable_parameters(model) | |
| self.setup_quantization_for_training_post_build(model) | |
| return model |
🤖 Prompt for AI Agents
In src/axolotl/loaders/adapters/builders/lora.py around lines 49 to 71,
build_model currently ignores the inference flag and always loads pretrained
PEFT adapters as trainable; thread an inference boolean through to
load_pretrained_adapter and call it with is_trainable=(not inference) (or pass
inference and compute is_trainable inside the loader), update the
load_pretrained_adapter signature and its internal PeftModel.from_pretrained
invocation to set is_trainable based on the passed inference flag, and ensure
any call sites within this file pass the inference parameter when invoking
load_pretrained_adapter so pretrained adapters respect inference mode.
| try: | ||
| builder = AdapterBuilderFactory.create_builder( | ||
| self.cfg.adapter, self.cfg | ||
| ) | ||
| lora_config = builder.build_config(self.model) | ||
| except Exception as e: | ||
| LOG.debug( | ||
| f"Builder pattern failed for config-only, falling back to legacy: {e}" | ||
| ) | ||
| _, lora_config = load_lora( | ||
| self.model, self.cfg, inference=False, config_only=True | ||
| ) | ||
| else: |
There was a problem hiding this comment.
Narrow exception handling and guard LoRA-only fallback.
Catching Exception swallows real errors (BLE001). Also, only fall back to legacy LoRA for lora/qlora; otherwise re-raise.
- try:
+ try:
builder = AdapterBuilderFactory.create_builder(
self.cfg.adapter, self.cfg
)
lora_config = builder.build_config(self.model)
- except Exception as e:
- LOG.debug(
- f"Builder pattern failed for config-only, falling back to legacy: {e}"
- )
- _, lora_config = load_lora(
- self.model, self.cfg, inference=False, config_only=True
- )
+ except (ValueError, ImportError, TypeError, AttributeError, RuntimeError) as e:
+ LOG.debug("Builder pattern failed for config-only (%s).", e)
+ if self.cfg.adapter in {"lora", "qlora"}:
+ _, lora_config = load_lora(
+ self.model, self.cfg, inference=False, config_only=True
+ )
+ else:
+ raise📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try: | |
| builder = AdapterBuilderFactory.create_builder( | |
| self.cfg.adapter, self.cfg | |
| ) | |
| lora_config = builder.build_config(self.model) | |
| except Exception as e: | |
| LOG.debug( | |
| f"Builder pattern failed for config-only, falling back to legacy: {e}" | |
| ) | |
| _, lora_config = load_lora( | |
| self.model, self.cfg, inference=False, config_only=True | |
| ) | |
| else: | |
| try: | |
| builder = AdapterBuilderFactory.create_builder( | |
| self.cfg.adapter, self.cfg | |
| ) | |
| lora_config = builder.build_config(self.model) | |
| except (ValueError, ImportError, TypeError, AttributeError, RuntimeError) as e: | |
| LOG.debug("Builder pattern failed for config-only (%s).", e) | |
| if self.cfg.adapter in {"lora", "qlora"}: | |
| _, lora_config = load_lora( | |
| self.model, self.cfg, inference=False, config_only=True | |
| ) | |
| else: | |
| raise | |
| else: |
🧰 Tools
🪛 Ruff (0.12.2)
384-384: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In src/axolotl/loaders/model.py around lines 379 to 391, narrow the broad
Exception catch and only use the legacy LoRA fallback for LoRA/QLoRA adapters:
replace the generic except Exception with a targeted exception for the
adapter/builder failure (e.g., the specific AdapterBuilder/validation exception
your project defines; if none exists, catch RuntimeError/ValueError/TypeError
selectively) and re-raise any other exceptions; additionally, before calling
load_lora guard the fallback with a check like if self.cfg.adapter in ("lora",
"qlora") so you only fall back for those adapters, and log the full exception
details in the debug/error message.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
1 participant
Refactor Loraconfig and model constructor
Description
Refactor Loraconfig and model constructor
refactor introduces a builder-based adapter loading pattern for lora configurations and model construction : )
Motivation and Context
#3111
How has this been tested?
as currently repo dosent have lora full coverage of lora specific all the current and tests introduced in 3147 passes
Summary by CodeRabbit