diff --git a/CHANGELOG.md b/CHANGELOG.md index 86f0078d..85bb7306 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -109,6 +109,9 @@ All notable changes to ForgeLM are documented here. ### Fixed +- SLERP now snapshots both merged adapter states before restoring or updating the base model, + so the first adapter's contribution is preserved and the result responds to the configured + weights instead of silently returning only the second adapter (CR-A13-001, #407). - Preserve top-level `--dry-run`, `--config`, `--offline` and `--output` values when a subcommand omits the same flag (CR-A05-001, #414). - Stop describing the human-approval gate as enforcing segregation of duties (English and Turkish): it records the trainer and approver identities and does not compare them, and both are self-asserted. The ISO 27001 and SOC 2 diff --git a/forgelm/merging.py b/forgelm/merging.py index 7819e527..0064fc42 100644 --- a/forgelm/merging.py +++ b/forgelm/merging.py @@ -196,13 +196,17 @@ def _slerp_merge(base_model, adapters): base_state = {k: v.clone() for k, v in base_model.state_dict().items()} model_a = PeftModel.from_pretrained(base_model, adapters[0]["path"]) - state_a = model_a.merge_and_unload().state_dict() + # Resetting the base and loading another adapter would overwrite shared storage. + state_a = {k: v.clone() for k, v in model_a.merge_and_unload().state_dict().items()} del model_a # Restore base model before loading second adapter base_model.load_state_dict(base_state, strict=True) + del base_state # not needed again; free a full model-sized copy before the second merge model_b = PeftModel.from_pretrained(base_model, adapters[1]["path"]) + # No clone needed: base_model is not touched again until the final load_state_dict below, + # by which point merged_state holds freshly computed tensors. state_b = model_b.merge_and_unload().state_dict() del model_b diff --git a/tests/test_merging_algos.py b/tests/test_merging_algos.py index e6270f8b..38626e0e 100644 --- a/tests/test_merging_algos.py +++ b/tests/test_merging_algos.py @@ -513,6 +513,60 @@ def test_normal_case_still_uses_slerp(self): assert not used_linear, "F-L-18: orthogonal vectors should use SLERP, not linear fallback" +class TestSlerpRealAdapters: + """Exercise PEFT's shared parameter storage with tiny local adapters.""" + + @staticmethod + def _base_model(): + model = torch.nn.Sequential(torch.nn.Linear(2, 2, bias=False)) + with torch.no_grad(): + model[0].weight.copy_(torch.eye(2)) + return model + + @staticmethod + def _save_adapter(path, axis): + from peft import LoraConfig, get_peft_model + + model = get_peft_model( + TestSlerpRealAdapters._base_model(), + LoraConfig(r=1, lora_alpha=1, target_modules=["0"]), + ) + layer = model.base_model.model[0] + with torch.no_grad(): + layer.lora_A["default"].weight.zero_() + layer.lora_B["default"].weight.zero_() + layer.lora_A["default"].weight[0, axis] = 1 + layer.lora_B["default"].weight[axis, 0] = 1 + model.save_pretrained(path) + + @pytest.mark.parametrize("weights", [(1, 1), (3, 1), (1, 3), (1, 0), (0, 1)]) + @pytest.mark.parametrize("reverse", [False, True]) + def test_merge_preserves_both_adapters_and_respects_weights(self, tmp_path, weights, reverse): + from forgelm.merging import _slerp_merge + + paths = [tmp_path / "a", tmp_path / "b"] + for axis, path in enumerate(paths): + self._save_adapter(path, axis) + expected_a = torch.diag(torch.tensor([2.0, 1.0])) + expected_b = torch.diag(torch.tensor([1.0, 2.0])) + if reverse: + paths.reverse() + expected_a, expected_b = expected_b, expected_a + adapters = [{"path": str(path), "weight": weight} for path, weight in zip(paths, weights)] + base = self._base_model() + + merged = _slerp_merge(base, adapters) + + t = weights[1] / sum(weights) + angle = math.acos(4 / 5) + expected = (math.sin((1 - t) * angle) * expected_a + math.sin(t * angle) * expected_b) / math.sin(angle) + assert merged is base + torch.testing.assert_close(merged[0].weight, expected) + if 0 < t < 1: + assert not torch.allclose(merged[0].weight, expected_a) + assert not torch.allclose(merged[0].weight, expected_b) + + class TestTiesDisjointMergeRenormalization: """The TIES 'Merge' step is the paper's disjoint merge: average only the sign-agreeing models, renormalizing by their weight sum. Without the