Repository: huggingface/transformers
Pull request: Regex post processing in loading

# What does this PR do?

The change introduced in https://github.com/huggingface/transformers/pull/43261 with `__setattr__` is quite dangerous, as `source_patterns` and `target_patterns` live together and cannot be unentangled from one another if there is a capturing group. 
Thus, changing one with `__setattr__` without changing the other, or simply changing the wrong one first can lead to very unexpected behavior and wrong pattern matching/replacing.
For this reason, it's better to reinstantiate an object directly in `peft` instead of switching the targets/sources.

Changed files:
filename: src/transformers/core_model_loading.py
patch: @@ -49,38 +49,6 @@
 logger = logging.get_logger(__name__)
 
 
-def process_target_pattern(pattern: str) -> tuple[str, str | None]:
-    """
-    Process a target pattern for reverse mapping (when targets become sources).
-
-    This handles several edge cases in checkpoint conversion mappings:
-    - Removes `^` prefix and `$` suffix (start/end of string anchors)
-    - Removes negative lookahead/lookbehind assertions
-    - Detects capturing groups and replaces them with `\\1` backreference
-
-    Args:
-        pattern: The target pattern to process for reverse mapping.
-
-    Returns:
-        A tuple of (processed_pattern, captured_group) where captured_group is
-        the original capturing group found (e.g., "(encoder|decoder)") or None.
-    """
-    # Some mapping contains `^` to notify start of string when matching -> remove it during reverse mapping
-    pattern = pattern.removeprefix("^")
-    # Some mapping contains `$` to notify end of string when matching -> remove it during reverse mapping
-    pattern = pattern.removesuffix("$")
-    # Remove negative lookahead/behind if any. This is ugly but needed for reverse mapping of
-    # Qwen2.5, Sam3, Ernie4.5 VL MoE!
-    pattern = re.sub(r"\(\?.+\)", "", pattern)
-    # Allow capturing groups in patterns, i.e. to add/remove a prefix to all keys (e.g. timm_wrapper, sam3)
-    capturing_group_match = re.search(r"\(.+?\)", pattern)
-    captured_group = None
-    if capturing_group_match:
-        captured_group = capturing_group_match.group(0)
-        pattern = pattern.replace(captured_group, r"\1", 1)
-    return pattern, captured_group
-
-
 def build_glob_alternation(
     globs: list[WeightRenaming | WeightConverter | str],
 ) -> tuple[re.Pattern, dict[str, str], dict[str, str]]:
@@ -508,6 +476,38 @@ def reverse_op(self) -> ConversionOps:
         return Force16BytesAlignment()
 
 
+def process_target_pattern(pattern: str) -> tuple[str, str | None]:
+    """
+    Process a target pattern for reverse mapping (when targets become sources).
+
+    This handles several edge cases in checkpoint conversion mappings:
+    - Removes `^` prefix and `$` suffix (start/end of string anchors)
+    - Removes negative lookahead/lookbehind assertions
+    - Detects capturing groups and replaces them with `\\1` backreference
+
+    Args:
+        pattern: The target pattern to process for reverse mapping.
+
+    Returns:
+        A tuple of (processed_pattern, captured_group) where captured_group is
+        the original capturing group found (e.g., "(encoder|decoder)") or None.
+    """
+    # Some mapping contains `^` to notify start of string when matching -> remove it during reverse mapping
+    pattern = pattern.removeprefix("^")
+    # Some mapping contains `$` to notify end of string when matching -> remove it during reverse mapping
+    pattern = pattern.removesuffix("$")
+    # Remove negative lookahead/behind if any. This is ugly but needed for reverse mapping of
+    # Qwen2.5, Sam3, Ernie4.5 VL MoE!
+    pattern = re.sub(r"\(\?.+\)", "", pattern)
+    # Allow capturing groups in patterns, i.e. to add/remove a prefix to all keys (e.g. timm_wrapper, sam3)
+    capturing_group_match = re.search(r"\(.+?\)", pattern)
+    captured_group = None
+    if capturing_group_match:
+        captured_group = capturing_group_match.group(0)
+        pattern = pattern.replace(captured_group, r"\1", 1)
+    return pattern, captured_group
+
+
 @dataclass(slots=True)
 class WeightTransform:
     source_patterns: str | list[str] = field(init=True)
@@ -520,52 +520,18 @@ class WeightTransform:
     collected_tensors: dict[str, list[Future]] = field(default_factory=lambda: defaultdict(list), init=False)
     layer_targets: dict[str, set[str]] = field(default_factory=lambda: defaultdict(set), init=False)
 
-    def __setattr__(self, name: str, value: Any):
-        if name == "source_patterns":
-            if isinstance(value, str):
-                value = [value]
-            normalized = []
-            for pattern in value:
-                if r"\1" in pattern:
-                    pattern = pattern.replace(r"\1", r"(.+)")
-                normalized.append(pattern)
-            object.__setattr__(self, name, normalized)
-            self._rebuild_compiled_sources()
-            return
-        if name == "target_patterns":
-            if isinstance(value, str):
+    def __setattr__(self, name, value):
+        if name in ("source_patterns", "target_patterns"):
+            # We do not allow to re-set the patterns, as they are linked between each other and changing one
+            # without the other can mess-up with the capturing groups/compiled sources
+            if hasattr(self, name):
+                raise ValueError(f"Cannot assign to field {name}, you should create a new instance")
+            # Switch str to list
+            elif isinstance(value, str):
                 value = [value]
-            normalized = []
-            for pattern in value:
-                # Some mapping contains `^` to notify start of string when matching -> remove it during reverse mapping
-                pattern = pattern.removeprefix("^")
-                # Some mapping contains `$` to notify end of string when matching -> remove it during reverse mapping
-                pattern = pattern.removesuffix("$")
-                # Remove negative lookahead/behind if any. This is ugly but needed for reverse mapping of
-                # Qwen2.5, Sam3, Ernie4.5 VL MoE!
-                pattern = re.sub(r"\(\?.+\)", "", pattern)
-                # Allow capturing groups in patterns, i.e. to add/remove a prefix to all keys (e.g. timm_wrapper, sam3)
-                if r"(.+)" in pattern:
-                    pattern = pattern.replace(r"(.+)", r"\1")
-                normalized.append(pattern)
-            object.__setattr__(self, name, normalized)
-            return
         object.__setattr__(self, name, value)
 
-    def _rebuild_compiled_sources(self):
-        branches = []
-        for i, source_pattern in enumerate(self.source_patterns):
-            group_name = f"g{i}"
-            pattern = source_pattern.replace(".*.", r"\..*\.")
-            branches.append(f"(?P<{group_name}>{pattern})")
-        object.__setattr__(self, "compiled_sources", re.compile("|".join(branches)))
-
     def __post_init__(self):
-        if isinstance(self.source_patterns, str):
-            self.source_patterns = [self.source_patterns]
-        if isinstance(self.target_patterns, str):
-            self.target_patterns = [self.target_patterns]
-
         # Due to how our `_checkpoint_conversion_mapping` mappings are written, we need a few exceptions here
         # when instantiating the reverse mapping (i.e. the targets become sources, and sources become targets)
         # The issues lie in the sources usually, so here we need to check the targets for the reversed mapping
@@ -601,13 +567,18 @@ def __post_init__(self):
             self.source_patterns[i] = pattern
 
         # Construct the regex we will use to rename keys from the sources to the targets
-        self._rebuild_compiled_sources()
+        branches = []
+        for i, source_pattern in enumerate(self.source_patterns):
+            group_name = f"g{i}"
+            pattern = source_pattern.replace(".*.", r"\..*\.")
+            branches.append(f"(?P<{group_name}>{pattern})")
+        self.compiled_sources = re.compile("|".join(branches))
 
     def add_tensor(self, target_key: str, source_key: str, source_pattern: str, future: Future):
         self.collected_tensors[source_pattern].append(future)
         self.layer_targets[target_key].add(source_key)
 
-    def rename_source_key(self, source_key: str) -> tuple[str, str | re.Pattern]:
+    def rename_source_key(self, source_key: str) -> tuple[str, str | None]:
         """
         Return a tuple (renamed_key, source_pattern_producing_the_match).
         Try renaming `source_key` according to the source and target patterns of the current WeightTransform.
@@ -729,6 +700,7 @@ class WeightConverter(WeightTransform):
     operations: list[ConversionOps] = field(default_factory=list, repr=False)
 
     def __post_init__(self):
+        WeightTransform.__post_init__(self)
         if bool(len(self.source_patterns) - 1) + bool(len(self.target_patterns) - 1) >= 2:
             # We allow many-to-many only if we use an internal operation that can handle it
             if not any(isinstance(op, _INTERNAL_MANY_TO_MANY_CONVERSIONS) for op in self.operations):

filename: src/transformers/integrations/peft.py
patch: @@ -229,10 +229,9 @@ def _build_peft_weight_mapping(
         if orig_conversion.target_patterns == ["mlp.experts.gate_up_proj"]:
             # gate_up_proj requires both merging the experts and concatenating for the fusion of w1 and w3
             for lora in ("lora_A", "lora_B"):  # TODO: lora_embedding_A and lora_embedding_B
-                conversion = copy.deepcopy(orig_conversion)
                 # deal with operations
                 peft_weight_operations = []
-                for i, op in enumerate(conversion.operations):
+                for op in orig_conversion.operations:
                     if isinstance(op, Concatenate):
                         if lora == "lora_B":  # block diagonal concat
                             peft_weight_operations.append(PeftConcatenate(dim=op.dim))
@@ -241,31 +240,37 @@ def _build_peft_weight_mapping(
                             peft_weight_operations.append(FlattenDims(dims=(0, 1)))
                     elif isinstance(op, MergeModulelist):
                         peft_weight_operations.append(op)
-                conversion.operations = peft_weight_operations
 
                 # TODO: this assumption may not hold for models != mixtral
                 # For source, we capture the orignal weights + the lora weights
                 new_source_patterns = []
-                for pat in list(conversion.source_patterns):
+                for pat in list(orig_conversion.source_patterns):
                     # we replace the weight pattern to colllect loras
                     pat = pat.rsplit(".", 1)[0]
                     # note: the source state_dict does *not* contain the adapter name
                     new_source_patterns.append(f"{pat}.{lora}.*")
-                conversion.source_patterns = new_source_patterns
 
                 # the gate_up_proj is the innner PEFT ParamWrapper, so we need to use base_layer
-                pat = conversion.target_patterns[0]
+                pat = orig_conversion.target_patterns[0]
                 pat = pat.replace("gate_up_proj", "base_layer")
                 # we make sure the target key is correct, add '.weight' because the parameter is targeted directly
-                conversion.target_patterns = [f"{pat}.{lora}.{adapter_name}.weight"]
-                new_weight_conversions.append(conversion)
+                new_target_patterns = [f"{pat}.{lora}.{adapter_name}.weight"]
+
+                # Instantiate a new object that correctly post process patterns if needed
+                new_conversion = orig_conversion.__class__(
+                    source_patterns=new_source_patterns,
+                    target_patterns=new_target_patterns,
+                    distributed_operation=orig_conversion.distributed_operation,
+                    quantization_operation=orig_conversion.quantizatin_operations,
+                    operations=new_weight_conversions,
+                )
+                new_weight_conversions.append(new_conversion)
 
         elif orig_conversion.target_patterns == ["mlp.experts.down_proj"]:
             # down_proj only requires merging of experts
             for lora in ("lora_A", "lora_B"):  # TODO: lora_embedding_A and lora_embedding_B
-                conversion = copy.deepcopy(orig_conversion)
                 peft_weight_operations = []
-                for i, op in enumerate(conversion.operations):
+                for op in orig_conversion.operations:
                     if isinstance(op, MergeModulelist):
                         peft_weight_operations.append(op)
                         if lora == "lora_A":
@@ -274,24 +279,31 @@ def _build_peft_weight_mapping(
                             peft_weight_operations.append(PermuteDims(dims=(2, 0, 1)))
                             peft_weight_operations.append(FlattenDims(dims=(0, 1)))
                             peft_weight_operations.append(Transpose(dim0=0, dim1=1))
-                conversion.operations = peft_weight_operations
 
                 # TODO: this assumption may not hold for models != mixtral
                 # For source, we capture the orignal weights + the lora weights
                 new_source_patterns = []
-                for pat in list(conversion.source_patterns):
+                for pat in list(orig_conversion.source_patterns):
                     # we replace the weight pattern to colllect loras
                     pat = pat.rsplit(".", 1)[0]
                     # note: the source state_dict does *not* contain the adapter name
                     new_source_patterns.append(f"{pat}.{lora}.*")
-                conversion.source_patterns = new_source_patterns
 
                 # the down_proj is the outer PEFT ParamWrapper, so we remove the prefix
-                pat = conversion.target_patterns[0]
+                pat = orig_conversion.target_patterns[0]
                 pat = pat.replace(".down_proj", "")
                 # we make sure the target key is correct, add '.weight' because the parameter is targeted directly
-                conversion.target_patterns = [f"{pat}.{lora}.{adapter_name}.weight"]
-                new_weight_conversions.append(conversion)
+                new_target_patterns = [f"{pat}.{lora}.{adapter_name}.weight"]
+
+                # Instantiate a new object that correctly post process patterns if needed
+                new_conversion = orig_conversion.__class__(
+                    source_patterns=new_source_patterns,
+                    target_patterns=new_target_patterns,
+                    distributed_operation=orig_conversion.distributed_operation,
+                    quantization_operation=orig_conversion.quantizatin_operations,
+                    operations=new_weight_conversions,
+                )
+                new_weight_conversions.append(new_conversion)
 
     return new_weight_conversions