Skip to content
Open
Changes from 1 commit
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
7 changes: 4 additions & 3 deletions src/llmcompressor/modifiers/awq/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from loguru import logger
from pydantic import ConfigDict, PrivateAttr, model_validator
from torch.nn import Module
from operator import attrgetter
from tqdm import tqdm

from llmcompressor.core import Event, EventType, State
Expand All @@ -29,7 +30,7 @@
from llmcompressor.pipelines.cache import IntermediatesCache
from llmcompressor.utils.fsdp.helpers import get_fsdp_parent
from llmcompressor.utils.helpers import calibration_forward_context
from llmcompressor.utils.pytorch.module import get_layer_by_name, get_layers
from llmcompressor.utils.pytorch.module import get_layers

__all__ = ["AWQModifier"]

Expand Down Expand Up @@ -323,7 +324,7 @@ def _set_resolved_mappings(self, model: Module) -> None:
smooth_layer = smooth_layers[smooth_name]

smooth_parent_name = ".".join(smooth_name.split(".")[:-1])
smooth_parent = get_layer_by_name(smooth_parent_name, model)
smooth_parent = attrgetter(smooth_parent_name)(model)
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

This line could raise an AttributeError if smooth_parent_name is an empty string. This can happen if smooth_name refers to a top-level module in model (i.e., its name does not contain any dots). In that case, smooth_name.split('.')[:-1] results in an empty list, and smooth_parent_name becomes "", which is not a valid input for attrgetter.

To prevent a crash, you should handle this case by assigning model as the parent when smooth_parent_name is empty, as this indicates smooth_name is a direct attribute of model.

Suggested change
smooth_parent = attrgetter(smooth_parent_name)(model)
smooth_parent = attrgetter(smooth_parent_name)(model) if smooth_parent_name else model

Copy link
Contributor Author

Choose a reason for hiding this comment

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

: )

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I'm a bit concerned this has the potential to error out as well. what if there's no parent?


balance_layers, balance_names = [], []
for balance_regex in mapping.balance_layers:
Expand Down Expand Up @@ -765,7 +766,7 @@ def get_lowest_common_parent(names: List[str], module: Module) -> Tuple[str, Mod
while True:
if parent_name == "":
return "", module
parent = get_layer_by_name(parent_name, module)
parent = attrgetter(parent_name)(module)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
parent = attrgetter(parent_name)(module)
parent = module.get_submodule(parent_name)

if not isinstance(parent, torch.nn.ModuleList):
return parent_name, parent
parent_name = ".".join(parent_name.split(".")[:-1])