fix(bnb): apply scaling to lora_bias merge/unmerge on 8-bit and 4-bit by Sravanjangam · Pull Request #3757 · huggingface/peft

Sravanjangam · 2026-09-16T18:20:52Z

Summary

Merging a LoRA adapter with lora_bias=True into bitsandbytes 8-bit/4-bit layers silently dropped the * scaling factor on the bias, while the base nn.Linear path applies it. Includes the wider scope from @yashdoke7 per @BenjaminBossan's direction to combine ("include those other fixes in the same PR").

Coordination / approval

Issue: bnb LoRA merge with lora_bias=True drops scaling factor on bias #3741 (proposed fix: two one-liners + CPU test)
@yashdoke7 scope expansion: bnb LoRA merge with lora_bias=True drops scaling factor on bias #3741 (comment)... — 4 sites (8-bit + 4-bit, merge + unmerge), .all() guard, no-bias guard, test-home question
@BenjaminBossan approval: "Please go ahead with your suggested PR ... Please include those other fixes in the same PR." — bnb LoRA merge with lora_bias=True drops scaling factor on bias #3741 (comment)...
Credit to @yashdoke7 for identifying the 4-bit twins, the .all() divergence, and the missing guard

Root cause

src/peft/tuners/lora/bnb.py: 8-bit merge did base.bias.data + lora_B.bias (L127) and unmerge bias.data -= lora_B.bias (L169); 4-bit twins at L404/L446 — none multiplied by self.scaling[adapter], unlike lora/layer.py:978 and forward (output = lora_B(lora_A(x)) * scaling scales the bias too). Earlier partial backport (FIX: Error when merging LoRA bias with scale != 1 #2489) fixed layer.py only.
safe_merge bias guards used not torch.isfinite(bias_data) without .all() → ambiguous-Boolean RuntimeError on multi-element bias instead of the intended NaN ValueError (layer.py uses .all()).
Both bnb merge paths lacked the RuntimeError guard layer.py raises when the base layer has no bias.

Fix (`src/peft/tuners/lora/bnb.py`, +20/-6)

4 bias sites now apply * self.scaling[active_adapter] (8-bit merge/unmerge, 4-bit merge/unmerge).
Both safe_merge bias guards now use .all().
Both merge paths raise RuntimeError("Impossible to merge LoRA with ... has no bias.") matching layer.py.

Tests (`tests/test_quantization.py`, +108)

test_bnb_8bit_merge_bias_applies_scaling — merge bias equals base + B.bias * 8.0, unmerge restores base, NaN safe_merge raises ValueError.
test_bnb_4bit_merge_bias_applies_scaling — same merge/unmerge parity on the 4-bit path.
test_bnb_merge_bias_without_base_bias_raises — no-bias base raises RuntimeError.
All CPU-runnable with weight-dequant stubbed (int8/4bit dequant needs CUDA; the bias formula under test is untouched). r=4, lora_alpha=32 (scaling 8.0) so a missing factor shows as max-abs ~3.35. Note on test home (replying to @yashdoke7's question): GPU parity tests live in test_common_gpu.py (test_8bit/4bit_merge_lora_with_bias use default scaling 1.0, which is why this never showed); these CPU tests pin the scaling factor where CI always runs. Happy to also bump those GPU configs to lora_alpha != r as a follow-up if wanted.

Verification

New tests 3/3 pass twice; 3/3 FAIL on db7d68a2 / PASS with fix (clean worktree + PYTHONPATH).
Issue repro: match True, maxabs 0.0, unmerge restores base.
Sibling test_custom_models.py -k "lora_bias or safe_merge or merge_with": 324 passed, 5 failed — all 5 are FrodConfig test_safe_merge [235-239], failing identically on base (stash-proven pre-existing).
ruff check + ruff format --check clean; doc-builder style --max_len 119 exit 0 on bnb.py.
Note per @BenjaminBossan's GPU comment: no GPU here, so bnb weight dequant is stubbed (CPU) and the tests assert the bias formula + guards, not quantized numerics; GPU numerics remain covered by the existing test_common_gpu.py merge tests.

Environment: Python 3.12, torch 2.13.0+cpu, bitsandbytes 0.50.2, transformers 5.15.1, peft @ c759e1ec.

@yashdoke7

Fixes huggingface#3741, includes scope from @yashdoke7, per @BenjaminBossan ('include those other fixes in the same PR'). - 8-bit + 4-bit merge/unmerge: bias now multiplied by self.scaling[active_adapter], matching lora/layer.py and forward (output = lora_B(lora_A(x)) * scaling scales bias too). - safe_merge bias guard: add missing .all() (was ambiguous-Boolean RuntimeError on multi-element bias instead of NaN ValueError). - merge: raise RuntimeError when base layer has no bias, like layer.py. Tests (tests/test_quantization.py, CPU-runnable with weight-dequant stub; bias formula untouched): - test_bnb_8bit_merge_bias_applies_scaling (merge + unmerge + NaN guard) - test_bnb_4bit_merge_bias_applies_scaling (merge + unmerge) - test_bnb_merge_bias_without_base_bias_raises Verification: - 3/3 pass twice; 3/3 FAIL on db7d68a / PASS with fix - sibling custom bias/merge 324 passed, 5 Frod pre-existing (stash-proven) - ruff check + format + doc-builder clean

Sravanjangam · 2026-09-16T18:27:27Z

On the GPU point: no GPU access here, so the bias formula is pinned by CPU tests with only the CUDA-only weight dequant stubbed (real merge/unmerge paths, scaling 8.0). Happy to bump the existing test_8bit/4bit_merge_lora_with_bias GPU tests to lora_alpha != r as a follow-up so GPU CI guards the numerics, or run a Kaggle repro if you'd prefer — just say which.

BenjaminBossan

Thanks for providing the fix. It looks good, but the testing needs some work. If you can't run the tests yourself, please let me know so that I can run them.

BenjaminBossan · 2026-09-21T13:52:21Z

+        # unlike the base path (`lora/layer.py`). CPU-runnable: int8 dequant needs CUDA, so
+        # only the weight dequant is stubbed — the bias formula under test is untouched.
+        # scaling 8.0 (r=4, lora_alpha=32) makes a missing factor show up as max-abs ~3.35.
+        import bitsandbytes as bnb


Generally, use global imports

Done — the moved tests need zero new imports (all already global in that module). The old stub tests are deleted; the no-bias RuntimeError test stays in test_quantization.py since it needs no GPU dequant (branch coverage the GPU tests cannot reach — every real layer has a bias).

BenjaminBossan · 2026-09-21T13:53:51Z

        check_outputs_similar(out_before, out_unloaded)
+
+    @pytest.mark.skipif(not is_bnb_available(), reason="bitsandbytes not available")
+    def test_bnb_8bit_merge_bias_applies_scaling(self):


These tests should live in test_gpu_examples.py in the PeftBnbGPUExampleTests class.

Done in 2895298e (plain push): both scaling tests moved to PeftBnbGPUExampleTests as test_bnb_8bit/4bit_merge_bias_applies_scaling, marked single_gpu_tests.

BenjaminBossan · 2026-09-21T13:54:23Z

+        bnb_lora.dequantize_bnb_weight = lambda w, state=None: w.data.float()
+        try:
+
+            class BnbNet(torch.nn.Module):


Use a real model like peft-internal-testing/opt-125m please.

Done — both tests load peft-internal-testing/opt-125m (verified enable_bias=True) with BitsAndBytesConfig + device_map='auto', selecting the first biased LoraLayer defensively.

BenjaminBossan · 2026-09-21T13:54:53Z

+        from peft import LoraConfig
+
+        orig_dequant = bnb_lora.dequantize_bnb_weight
+        bnb_lora.dequantize_bnb_weight = lambda w, state=None: w.data.float()


The test should work without monkeypatching this function. I think using a real Transformers model should be enough for that.

Done — no stubs or monkeypatching anymore. The 8-bit/4-bit merge runs genuine dequant on the quantized model; assertions target the bias formula only.

…eal opt-125m

Sravanjangam · 2026-09-21T18:14:52Z

@BenjaminBossan : One flag on the moved GPU tests (test_bnb_8bit/4bit_merge_bias_applies_scaling in PeftBnbGPUExampleTests): I have no CUDA machine here, so I could only verify collection, ruff, and the logic by inspection I could not execute them. Could you please run those two tests on your side? Everything else in the PR is verified locally.

BenjaminBossan

Thanks for the updates, but there are still issues with the testing. Please check my comments.

Moreover, I noticed that the tests have no empty lines at all. Please add some empty lines to group code that belongs together, check the surrounding tests for inspiration. I noticed that the new OpenAI models have a tendency to write very dense code, are you using those?

BenjaminBossan · 2026-09-22T10:09:59Z

+    def test_bnb_8bit_merge_bias_applies_scaling(self):
+        # Regression test for #3741: the 8-bit merge/unmerge dropped
+        # `* scaling` on the bias, unlike the base path (`lora/layer.py`).
+        # Runs on a real quantized model, so the dequant path is genuine.


Suggested change

# Runs on a real quantized model, so the dequant path is genuine.

Makes no sense to comment on that. Same for the test below.

Fixed in b1627a9 — the comment block was condensed to one clear line, and both tests now group code with blank lines.

BenjaminBossan · 2026-09-22T10:12:21Z

+        # Runs on a real quantized model, so the dequant path is genuine.
+        # scaling 8.0 (r=4, lora_alpha=32) makes a missing factor show up.
+        model_id = "peft-internal-testing/opt-125m"
+        model = AutoModelForCausalLM.from_pretrained(


Please always load the models inside the hub_online_once context.

Done in b1627a9 — both tests now load peft-internal-testing/opt-125m inside hub_online_once(model_id).

BenjaminBossan · 2026-09-22T10:15:07Z

        check_outputs_similar(out_before, out_unloaded)
+
+    @pytest.mark.skipif(not is_bnb_available(), reason="bitsandbytes not available")
+    def test_bnb_merge_bias_without_base_bias_raises(self):


test_quantization is for testing the generic quantization backend in PEFT, which LoRA does not use. Therefore, delete this test. I also don't see what this test would cover that isn't already covered by the other tests.

Deleted in b1627a9 — the no-bias RuntimeError test was removed from test_quantization.py; the migration + no-bias-path coverage now lives entirely in test_gpu_examples.py PeftBnbGPUExampleTests.

@BenjaminBossan

…ternals Per @BenjaminBossan review on huggingface#3757: - Compare logits pre/post merge and pre/post unmerge (MSE < 1.0) on a real quantized opt-125m instead of checking lora_B bias + scaling internals. - Load the model inside hub_online_once. - Group code with blank lines. - Delete test_bnb_merge_bias_without_base_bias_raises from test_quantization.py (generic quantization backend file; LoRA tests live in test_gpu_examples.py).

BenjaminBossan

Thanks for cleaning up the tests, I still have some comments about them, please take a look.

BenjaminBossan · 2026-09-28T10:10:10Z

+
+        # With `lora_bias=True`, merging must reproduce the LoRA contribution including `* scaling`
+        # (scaling is 8.0 for r=4, lora_alpha=32). A dropped factor shifts the logits by several units.
+        mse = ((out_before - out_merged) ** 2).mean()


Any specific reason why you check MSE and not call torch.allclose the logits?

BenjaminBossan · 2026-09-28T10:10:37Z

+
+        mse = ((out_before - out_unmerged) ** 2).mean()
+        assert mse < 1.0
+


This is missing the safe merge check that exists in the 8bit test.

… cleanups - Drop the LoRA bnb bias scaling and its test, which huggingface#3757 covers; the bnb merge now only computes and checks the bias before writing the weight. - Clarify which layer the broken adapter test poisons and note that earlier layers stay merged if a later one fails. - torchao several adapters test: link huggingface#3728, load the model once inside hub_online_once and deepcopy it.

BenjaminBossan · 2026-10-01T09:51:40Z

@Sravanjangam Are you still working on this?

Sravanjangam · 2026-10-01T12:21:17Z

@Sravanjangam Are you still working on this?

yes

BenjaminBossan · 2026-10-12T23:30:01Z

Gentle ping @Sravanjangam. LMK if you don't have time to work on this, so someone else can take over.

Sravanjangam · 2026-10-13T05:24:24Z

Gentle ping @Sravanjangam. LMK if you don't have time to work on this, so someone else can take over.

I will finish this today

- Wrap model loading in hub_online_once context - Check output parity with torch.allclose on logits instead of direct bias comparison or MSE - Add safe_merge NaN guard to 4-bit test (matching 8-bit test) - Delete test_bnb_merge_bias_without_base_bias_raises from test_quantization.py (LoRA does not use generic quantization backend) Per BenjaminBossan's review comments on huggingface#3757 (09-21, 09-28).

Sravanjangam · 2026-10-16T18:46:23Z

Hi @BenjaminBossan — answering both of your points; changes are in d5b7a58f and 616b258c.

1. MSE vs torch.allclose on the logits

You were right that MSE was the wrong check, and I also found that torch.allclose directly on the logits does not work here: bnb re-quantizes the weight during merge, so the merged and unmerged outputs are never bit-identical, and at atol=1e-3 the assertion fails even with the correct scaling. Measured on a CPU probe over 5 seeds (no GPU here, so the 8-bit path with a stubbed dequant; values are max-abs in softmax space):

	max-abs softmax diff
re-quantization noise alone (correct scaling)	1.3e-03 – 4.1e-03
dropped `* scaling` factor (pre-fix formula)	1.6e-01 – 3.8e-01

The comparison is therefore done in probability space — torch.allclose(F.softmax(out_before), F.softmax(out_merged), atol=1e-2, rtol=1e-2) — which is the same space the existing test_8bit_merge_lora_with_bias / test_4bit_merge_lora_with_bias compare in. Both directions were checked against the pre-fix formula: 5/5 pass with the fix, 5/5 fail without it, so the test pins the bug and not the quantization noise. Also added the global torch.nn.functional import (the tests no longer import locally).

2. Safe merge check in the 4-bit test

Added the same guard the 8-bit test has: fill lora_B["default"].bias with NaN, then expect ValueError matching "NaNs detected" from merge_adapter(safe_merge=True).

3. Two things I found while re-reading the tests (616b258c)

Added a sanity check that lora_B's bias is non-zero before the parity assertions: with a zero bias the merged and unmerged outputs would agree for any scaling, so the regression assertions would pass vacuously. This follows the same pattern as the sanity check you asked for in fix(delete): restore fallback trainability after deleting active adapter #3743.
Corrected the docstring, which claimed the test proves scaling on both merge and unmerge. Only the merged-vs-unmerged comparison pins the bug; the round trip just shows unmerge inverts merge, and the pre-fix formula satisfied that too since it was wrong in both directions.

One caveat worth flagging: the tolerance was calibrated on the 8-bit path. The 4-bit test uses the same comparison, and 4-bit re-quantization is coarser, so if that constant turns out too tight on the GPU runner it is a single number to adjust — I could not measure the 4-bit path locally since it needs CUDA.

torch.allclose on the raw logits at atol=1e-3 fails even with the correct scaling: bnb re-quantizes the weight on merge, so the outputs are never bit-identical. Measured on a CPU probe over 5 seeds, the re-quantization alone moves the softmax by ~4e-03 max-abs while the dropped factor moves it by ~2e-01, so the comparison is done in probability space with atol=1e-2/rtol=1e-2: 5/5 pass with the fix, 5/5 fail with the pre-fix formula, so the test still pins the bug. Also add the global torch.nn.functional import (F was imported locally inside the tests before).

- Assert the LoRA bias is non-zero before the parity checks: with a zero bias the merged and unmerged outputs agree for any scaling, so the regression assertions would pass vacuously. - Correct the comment: the merged-vs-unmerged comparison is what pins the bug; the round trip only checks that unmerge inverts merge, which the pre-fix formula satisfied too (it was wrong in both directions). - Reuse the layer lookup from the sanity check instead of looking it up twice.

BenjaminBossan requested changes Sep 21, 2026

View reviewed changes

test(gpu): move bnb bias-scaling tests to PeftBnbGPUExampleTests on r…

2895298

…eal opt-125m

BenjaminBossan requested changes Sep 22, 2026

View reviewed changes

BenjaminBossan requested changes Sep 28, 2026

View reviewed changes

BenjaminBossan mentioned this pull request Sep 28, 2026

FIX safe_merge leaving the base layer modified, and merging several adapters at once on torchao #3779

Open

Sravanjangam added 2 commits October 17, 2026 00:39


		mse = ((out_before - out_unmerged) ** 2).mean()
		assert mse < 1.0

Conversation

Sravanjangam commented Sep 16, 2026

Summary

Coordination / approval

Root cause

Fix (src/peft/tuners/lora/bnb.py, +20/-6)

Tests (tests/test_quantization.py, +108)

Verification

Uh oh!

Sravanjangam commented Sep 16, 2026

Uh oh!

BenjaminBossan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sravanjangam commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BenjaminBossan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BenjaminBossan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BenjaminBossan commented Oct 1, 2026

Uh oh!

Sravanjangam commented Oct 1, 2026

Uh oh!

BenjaminBossan commented Oct 12, 2026

Uh oh!

Sravanjangam commented Oct 13, 2026

Uh oh!

Sravanjangam commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fix (`src/peft/tuners/lora/bnb.py`, +20/-6)

Tests (`tests/test_quantization.py`, +108)

Sravanjangam commented Sep 21, 2026 •

edited

Loading

Sravanjangam commented Oct 16, 2026 •

edited

Loading