feat: support LoRA conversion for multiplicative PEFT methods (OFT, BOFT)#3360
feat: support LoRA conversion for multiplicative PEFT methods (OFT, BOFT)#3360peft-jambot wants to merge 7 commits into
Conversation
…OFT) Add get_additive_delta method to BaseTunerLayer that returns the additive delta W' - W for any PEFT method. The default implementation delegates to get_delta_weight (correct for all additive methods like LoRA, LoKr, LoHa). Override get_additive_delta in OFT and BOFT Linear layers to compute the actual additive delta by constructing the effective weight W' from the multiplicative transformation and subtracting the base weight W. Update conversion.py to call get_additive_delta instead of get_delta_weight, enabling LoRA conversion for multiplicative methods. Enable supports_lora_conversion on OFT and BOFT Linear layers. Key math: - OFT: W' = W @ R^T, delta = W @ R^T - W - BOFT: W' = (W @ R^T) * s, delta = (W @ R^T) * s - W Tests: Added TestMultiplicativeLoraConversion class in test_lora_conversion.py with 7 tests covering OFT and BOFT conversion (supports_lora_conversion, approximation quality, targeted modules identity, save_as_lora round-trip). All 38 tests in test_lora_conversion.py pass. 1788 OFT/BOFT tests in test_custom_models.py pass.
BenjaminBossan
left a comment
BenjaminBossan
left a comment
There was a problem hiding this comment.
Thanks for the PR. I've added some comments, please check.
| """ | ||
| Return the additive delta weight W' - W for the given adapter. | ||
|
|
||
| For additive PEFT methods (e.g. LoRA, LoKr, LoHa), this is equivalent to ``get_delta_weight``. For |
There was a problem hiding this comment.
Please always use single backticks in all docstrings.
| def test_boft_supports_lora_conversion(self, boft_model): | ||
| assert boft_model.supports_lora_conversion() | ||
|
|
||
| def test_oft_converted_lora_approximates_original(self, oft_model): |
There was a problem hiding this comment.
Isn't this already covered by _test_lora_conversion in testing_common.py?
| mse_converted = self.get_mse(output_converted, output_oft) | ||
| assert 0.0 < mse_converted < 0.1, f"OFT conversion MSE too high: {mse_converted}" | ||
|
|
||
| def test_boft_converted_lora_approximates_original(self, boft_model): |
There was a problem hiding this comment.
Isn't this already covered by _test_lora_conversion in testing_common.py?
| mse_converted = self.get_mse(output_converted, output_boft) | ||
| assert 0.0 < mse_converted < 0.1, f"BOFT conversion MSE too high: {mse_converted}" | ||
|
|
||
| def test_oft_targeted_modules_identical(self, oft_model): |
There was a problem hiding this comment.
This test is unnecessary.
| modules_lora = {k.rsplit(".", 2)[0] for k in lora_state_dict.keys() if ".lora" in k} | ||
| assert modules_oft == modules_lora | ||
|
|
||
| def test_boft_targeted_modules_identical(self, boft_model): |
There was a problem hiding this comment.
This test is unnecessary.
| modules_lora = {k.rsplit(".", 2)[0] for k in lora_state_dict.keys() if ".lora" in k} | ||
| assert modules_boft == modules_lora | ||
|
|
||
| def test_oft_save_as_lora(self, oft_model, tmp_path): |
There was a problem hiding this comment.
This test is unnecessary.
… override in additive methods Address review feedback on PR huggingface#3360: - Change BaseTunerLayer.get_additive_delta to raise NotImplementedError by default instead of calling get_delta_weight, since not all PEFT methods are additive - Add get_additive_delta override to each additive PEFT method that supports LoRA conversion (LoRA, LoKr, LoHa, Vera, DeLoRA, RandLora, VBLoRA, C3A, Psoft, GraLoRA, FourierFT, WaveFT, TinyLora) - Fix double backticks to single backticks in docstrings - Remove 5 unnecessary tests already covered by _test_lora_conversion in testing_common.py
Review feedback addressedThanks for the review! I've addressed all comments in commit 176f598:
All tests pass:
|
BenjaminBossan
left a comment
BenjaminBossan
left a comment
There was a problem hiding this comment.
Thanks for the updates. There is still a bit of work to do, please check my inline comments and the discussion below.
As a bit of a clean up, could you please remove the supports_lora_conversion methods from HiRA and SHiRA, as they just return False and are thus unnecessary.
Technically speaking, MiSS doesn't require get_additive_delta because its conversion is special cased. But I think it would still be good to implement it. IIUC, when initializing MiSS with "bat", the delta_weight is already the additive delta_weight, check the code in conversion.py::_convert_miss_module_to_lora. So this case can be moved to get_additive_delta and then _convert_miss_module_to_lora only needs to be called when not using "bat", otherwise we just use the normal conversion path.
For non-bat, we still need the special code from _convert_miss_module_to_lora, but it can be simplified once the "bat" case has been factored out.
| def test_oft_supports_lora_conversion(self, oft_model): | ||
| assert oft_model.supports_lora_conversion() | ||
|
|
||
| def test_boft_supports_lora_conversion(self, boft_model): | ||
| assert boft_model.supports_lora_conversion() |
There was a problem hiding this comment.
These tests aren't really helpful. Let's delete them.
There was a problem hiding this comment.
Deleted these tests in commit 6a4247a. The TestMultiplicativeLoraConversion class is now empty and has been removed as well.
There was a problem hiding this comment.
In testing_common.py::_test_lora_conversion, we currently early return for PEFT methods that don't support LoRA conversion. That makes it hard to see which methods were actually tested and which ones just early returned. Please change this to use pytest.skip with an appropriate message instead.
There was a problem hiding this comment.
Changed to use pytest.skip with a message like 'XxxConfig does not support LoRA conversion' in commit 6a4247a. Unsupported methods now show up as SKIPPED instead of silently passing.
- Remove unnecessary supports_lora_conversion overrides from HiRA and SHiRA that just return False (base class default) - Implement get_additive_delta for MiSS bat mode, factoring the bat case out of _convert_miss_module_to_lora into the generic get_additive_delta + SVD path; _convert_miss_module_to_lora now only handles standard/mini - Replace early return with pytest.skip in _test_lora_conversion so unsupported PEFT methods show as skipped with a clear message - Delete unhelpful supports_lora_conversion assertion tests for OFT/BOFT and the now-empty TestMultiplicativeLoraConversion class - Add get_additive_delta to UniLora (new upstream tuner that declares supports_lora_conversion=True but was missing the method)
- Remove unnecessary supports_lora_conversion overrides from HiRA and SHiRA that just return False (base class default) - Implement get_additive_delta for MiSS bat mode, factoring the bat case out of _convert_miss_module_to_lora into the generic get_additive_delta + SVD path; _convert_miss_module_to_lora now only handles standard/mini - Replace early return with pytest.skip in _test_lora_conversion so unsupported PEFT methods show as skipped with a clear message - Delete unhelpful supports_lora_conversion assertion tests for OFT/BOFT and the now-empty TestMultiplicativeLoraConversion class - Add get_additive_delta to UniLora (new upstream tuner that declares supports_lora_conversion=True but was missing the method)
Review feedback addressedThanks for the review! I've addressed all comments in commit 6a4247a:
All tests pass:
|
2 participants
Description
Add support for converting multiplicative PEFT methods (OFT, BOFT) to LoRA via
convert_to_lora. Previously, only additive methods (whereW' = W + ΔW) could be converted, becauseconvert_to_lorausedget_delta_weight()to obtain the delta and SVD-approximate it. Multiplicative methods like OFT (W' = W @ R) and BOFT (W' = diag(s) @ W @ R) return a transformation matrix fromget_delta_weight(), not an additive delta.This PR introduces a
get_additive_delta()method onBaseTunerLayerthat computes the actual additive deltaW' - Wfor any PEFT method. For additive methods, the default implementation simply delegates toget_delta_weight(). For multiplicative methods, the method is overridden to construct the effective weightW'and subtract the base weightW.Fork issue: peft-jambot#4
Changes
src/peft/tuners/tuners_utils.py: Addedget_additive_delta()method toBaseTunerLayer. Default implementation returnsget_delta_weight(adapter_name), which works for all existing additive methods with zero changes.src/peft/tuners/oft/layer.py: Overrideget_additive_delta()forOFT.Linear— computesW @ R^T - WwhereRis the orthogonal rotation matrix. Also enablessupports_lora_conversion() = True.src/peft/tuners/boft/layer.py: Overrideget_additive_delta()forBOFT.Linear— computes(W @ R^T) * s - WwhereRis the rotation matrix andsis the scaling vector. Also enablessupports_lora_conversion() = True.src/peft/tuners/lora/conversion.py: Updated_convert_module_to_lora()andconvert_to_lora()to callget_additive_delta()instead ofget_delta_weight(). Updated docstrings and error messages accordingly.tests/test_lora_conversion.py: AddedTestMultiplicativeLoraConversionclass with 7 tests covering OFT and BOFT conversion:supports_lora_conversionchecks, approximation quality (MSE < 0.1), targeted module mapping, andsave_as_loraround-trip.How I tested
pytest tests/test_lora_conversion.py -v→ 38 passed (31 existing + 7 new)pytest tests/test_custom_models.py -k "OFT and not Quant"→ 1788 passed, 52 skippedpytest tests/test_custom_models.py -k "BOFT"→ 557 passed, 13 skippedmake style→ all checks passedNo GPU/quantization tests were run, as the changes only affect the LoRA conversion code path, which operates on dequantized weights.
AI assistance statement
This PR was prepared with AI assistance (Hermes Agent). The human submitter reviewed every changed line and ran the relevant tests.