fix: store miss_fn per-adapter in MiSS layer by peft-jambot · Pull Request #3379 · huggingface/peft

peft-jambot · 2026-07-01T12:46:04Z

Description

The miss_fn attribute (derived from init_weights) was stored as a single value on the MissLinear instance instead of per-adapter. This meant adding a second MiSS adapter with a different init_weights value (e.g. True vs "bat") would override the first adapter's setting, leading to incorrect behavior in merge, unmerge, forward, and LoRA conversion operations.

Changes

Convert self.miss_fn from a single attribute to a dict keyed by adapter_name, following the same pattern used by miss_r, miss_mini_r, and other per-adapter attributes.
Set self.miss_fn[adapter_name] in update_layer (which is called for each adapter) instead of in MissLinear.__init__ (called only once).
Add miss_fn to other_param_names so it is included in adapter lifecycle operations (delete_adapter, _all_available_adapter_names, etc).
Update all references to self.miss_fn in merge, unmerge, get_delta_weight_miss, forward, and the LoRA conversion code to use per-adapter lookup.

Tests

Added TestMissInitialization class to tests/test_initialization.py with three tests:

test_miss_fn_per_adapter: verifies miss_fn is stored per-adapter when adding two adapters with different init_weights values.
test_miss_fn_per_adapter_forward: ensures forward pass works correctly with different init_weights per adapter.
test_miss_fn_per_adapter_three_variants: tests all three init_weights variants (True, "bat", "mini") coexisting on the same layer.

Test results

python -m pytest tests/test_initialization.py::TestMissInitialization -x -v
# 3 passed

python -m pytest tests/test_custom_models.py -k "miss and not adm" -x -v
# 378 passed, 10 skipped

python -m pytest tests/test_lora_conversion.py -k "miss" -x -v
# 6 passed

AI assistance

This PR was created with AI assistance. The task was assigned via the peft-jambot issue tracker (issue #6). The changes were reviewed and tested locally.

The miss_fn attribute (derived from init_weights) was stored as a single value on the MissLinear instance instead of per-adapter. This meant adding a second adapter with a different init_weights value would override the first adapter's setting, leading to incorrect behavior in merge, unmerge, forward, and LoRA conversion operations. Convert miss_fn to a dict keyed by adapter_name, following the same pattern used by miss_r, miss_mini_r, and other per-adapter attributes. Add miss_fn to other_param_names so it is included in adapter lifecycle operations (delete_adapter, _all_available_adapter_names, etc). Fixes #6

BenjaminBossan

Thanks for this fix but I'm not convinced about how the forward pass is handled. Please check my comment.

BenjaminBossan · 2026-07-02T11:54:58Z

-            if self.miss_fn == "bat":
+            # Determine the MiSS variant from the active adapters. When multiple adapters are active, they must
+            # all use the same variant; otherwise, the forward pass is ambiguous.
+            active_fns = {self.miss_fn[adapter] for adapter in self.active_adapters if adapter in self.miss_block}


This looks incorrect. If I have a mix of bat and non-bat MiSS adapters, I would treat all of them as non-bat. Shouldn't it be possible to mix bat and non-bat in the same forward pass? If it's not possible, I would rather raise an error when incompatible MiSS adapters are being added.

Good point, thanks. Fixed in commit 136910c.

The forward pass now separates active adapters into bat and non-bat lists. bat adapters modify the base weight first (via get_delta_weight), then non-bat adapters (balance/mini) add their delta to the output. Both contributions are applied when mixed.

Two tests added (test_miss_mix_bat_and_non_bat_forward, test_miss_mix_bat_and_mini_forward) verifying that mixed forward passes produce outputs distinct from either adapter alone.

Please re-review when you have a chance.

Address review feedback on PR huggingface#3379. Instead of treating all adapters as non-bat when a mix of bat and non-bat adapters is active, the forward pass now separates them: bat adapters modify the base weight first, then non-bat adapters add their delta to the output. This allows bat and non-bat MiSS adapters to coexist in the same forward pass. Added two tests verifying that mixed bat+non-bat and bat+mini forward passes produce outputs that differ from either adapter used alone.

peft-jambot · 2026-07-02T12:07:03Z

Addressing review feedback

Thanks for the review! I've addressed the inline comment on the forward pass in commit 136910c.

The problem

The previous code checked active_fns == {"bat"} and fell through to the non-bat else branch for any mix, effectively treating all active adapters as non-bat — including bat adapters, whose weight-modification logic would be silently skipped.

The fix

Instead of choosing one branch based on a set comparison, the forward pass now separates active adapters into bat and non-bat lists and applies both:

bat adapters are applied first: they sequentially modify a clone of the base weight via get_delta_weight, then F.linear is called with the modified weight.
non-bat adapters (balance/mini) are applied on top: their output deltas are added to the result from step 1.

This means bat and non-bat MiSS adapters can now coexist in the same forward pass — bat modifies the weight, non-bat adds to the output, and both contributions are present.

Tests

Added two tests in test_initialization.py:

test_miss_mix_bat_and_non_bat_forward — activates a bat and a balance (default) adapter simultaneously, sets non-zero weights, and verifies the mixed output differs from either adapter used alone.
test_miss_mix_bat_and_mini_forward — same but with bat + mini.

All existing tests pass:

tests/test_initialization.py -k TestMissInitialization: 5 passed
tests/test_custom_models.py -k "miss and not dismiss": 378 passed, 10 skipped
tests/test_lora_conversion.py -k "miss": 6 passed
tests/test_custom_models.py -k "miss and (merge or unmerge)": 36 passed

BenjaminBossan

Thanks for the update to forward, I just have a small comment concerning the testing.

BenjaminBossan · 2026-07-03T12:56:25Z

+        assert layer.miss_fn["adapter1"] == "bat"
+        assert layer.miss_fn["adapter2"] == "mini"
+
+    def test_miss_mix_bat_and_non_bat_forward(self, data):


This test and test_miss_mix_bat_and_mini_forward don't really check if the different inits are correctly applied and as such don't add too much, except for guarding against the incorrect implementation that you initially submitted. Let's remove these tests.

Removed both tests in commit ba1f073. Thanks for the feedback.

Remove test_miss_mix_bat_and_non_bat_forward and test_miss_mix_bat_and_mini_forward per reviewer feedback. These tests only guarded against the incorrect initial implementation and don't verify that different inits are correctly applied.

peft-jambot · 2026-07-03T13:07:17Z

Review feedback addressed

Thanks for the feedback! I've removed both test_miss_mix_bat_and_non_bat_forward and test_miss_mix_bat_and_mini_forward in commit ba1f073, as you noted they only guarded against the incorrect initial implementation and don't verify that different inits are correctly applied.

The remaining MiSS initialization tests still pass:

test_miss_fn_per_adapter
test_miss_fn_per_adapter_forward
test_miss_fn_per_adapter_three_variants

HuggingFaceDocBuilderDev · 2026-07-07T12:21:05Z

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

BenjaminBossan

LGTM, failing CI is unrelated. @githubnemo please review.

github-actions · 2026-08-01T15:23:33Z

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

…ialization.py - layer.py: keep per-adapter miss_fn dict logic, add quantization_backend supports_merge check from upstream for bat init_weights - test_initialization.py: keep both TestTinyLoraInitialization (upstream) and TestMissInitialization (this PR) classes

BenjaminBossan · 2026-08-20T15:57:51Z

not stale

githubnemo

One comment, otherwise LGTM

githubnemo · 2026-08-13T14:40:16Z

+        layer = model.base_model.model.lin0
+        assert layer.miss_fn["default"] is True
+        assert layer.miss_fn["adapter1"] == "bat"
+        assert layer.miss_fn["adapter2"] == "mini"


It would be good to also have a test that reproduces the overriding behavior this PR fixes.

Idea: have two adapters (one bat, one with a different init fn), gather per-adapter outputs, add both adapters into one model, trigger init fn override bug, observe that output != (adapter1_output + adpater2_output).

peft-jambot · 2026-09-10T14:47:30Z

Addressed the review feedback. Summary of changes:

config.py: Reworded the init_weights help text — False is now described as mainly for debugging purposes instead of the vague "don't change this setting".
layer.py: Fixed the mini_r ValueError message — it was a plain string with a non-interpolated {self.out_features} placeholder; now an f-string that actually prints the value, plus proper spacing between the concatenated segments.
tests/test_initialization.py:
- Changed the MLP fixture dims to 12→12 and 12→6 (so r=6/mini_r=2 divide cleanly) and seeded get_model/data.
- Removed the weak test_miss_fn_per_adapter_forward (only checked output shapes).
- Added test_miss_fn_output_respects_init_weights, which verifies that each init_weights option (True, False, "bat", "mini") produces a different output, and that a single model holding all four adapters produces the same per-adapter outputs as the individually-created models.

Note: the sanity-check assertion in the suggested diff referenced an undefined msg variable; I replaced it with an explicit assertion message.

Tests run:

pytest tests/test_initialization.py -k Miss → 5 passed
pytest tests/test_initialization.py tests/test_custom_models.py tests/test_config.py -k miss → 427 passed, 14 skipped
make style (ruff 0.15.12, matching setup.py) → passes

Ready for review again.

BenjaminBossan · 2026-09-14T10:56:16Z

@githubnemo Please review again. Note that the latest changes are code that I wrote and then passed as a patch to the bot, who made small adjustments.

peft-jambot · 2026-09-14T11:58:20Z

Synced and merged the latest main into this PR, and reconciled the tests.

What changed:

Merged upstream main (which now contains FIX Reject MiSS and PSOFT ranks the layer cannot support #3708, "Reject MiSS and PSOFT invalid ranks"). That PR added its own TestMissInitialization class with test_miss_rank_exceeds_bound_raises.
Since that upstream class already exists, this PR previously would have added a second TestMissInitialization class. I moved this PR's three tests (test_miss_fn_per_adapter, test_miss_fn_per_adapter_three_variants, test_miss_fn_output_respects_init_weights) into the existing class and removed the duplicate.
To make the two sets of tests share one get_model fixture (which is Linear(10, 30) / Linear(30, 2)), I adapted the output-respects test to r=2, mini_r=2 (the largest rank dividing both layers for bat) and added a data fixture of shape (4, 10) matching lin0's in_features=10. Also seeded get_model so the cross-model determinism check (single-option output == multi-adapter output) holds.

Tests run:

pytest tests/test_initialization.py -k Miss → 13 passed (includes both the rank-bound tests and this PR's per-adapter tests)
pytest tests/test_initialization.py tests/test_custom_models.py tests/test_config.py tests/test_lora_conversion.py -k "miss or (lora_conversion and miss)" → 506 passed, 14 skipped (covers the merged miss/layer.py and lora/conversion.py)
make style with ruff 0.16.4 (the version now pinned in setup.py) → passes, no unrelated files touched

Ready for review again.

githubnemo

Thanks for addressing :) LGTM

BenjaminBossan requested changes Jul 2, 2026

View reviewed changes

BenjaminBossan requested changes Jul 3, 2026

View reviewed changes

BenjaminBossan marked this pull request as ready for review July 7, 2026 12:55

BenjaminBossan approved these changes Jul 7, 2026

View reviewed changes

chore: trigger CI re-run after transient tiny-sd-pipe model outage

4e9884f

github-actions Bot closed this Aug 11, 2026

BenjaminBossan reopened this Aug 11, 2026

github-actions Bot closed this Aug 20, 2026

BenjaminBossan reopened this Aug 20, 2026

githubnemo reviewed Sep 10, 2026

View reviewed changes

test: sharpen MiSS init_weights tests and fix messages

9e90294

BenjaminBossan requested a review from githubnemo September 14, 2026 10:55

peft-jambot added 2 commits September 14, 2026 11:55

Merge commit 'a5e4c58a' into FIX-miss-per-adapter-fn-v2

257ab8f

test: merge PR MiSS tests into existing TestMissInitialization class

deb4e33

githubnemo approved these changes Sep 17, 2026

View reviewed changes

Conversation

peft-jambot commented Jul 1, 2026

Description

Changes

Tests

Test results

AI assistance

Uh oh!

BenjaminBossan left a comment

Choose a reason for hiding this comment

Uh oh!

BenjaminBossan Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

peft-jambot Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

peft-jambot commented Jul 2, 2026

Addressing review feedback

The problem

The fix

Tests

Uh oh!

BenjaminBossan left a comment

Choose a reason for hiding this comment

Uh oh!

BenjaminBossan Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

peft-jambot Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

peft-jambot commented Jul 3, 2026

Review feedback addressed

Uh oh!

HuggingFaceDocBuilderDev commented Jul 7, 2026

Uh oh!

BenjaminBossan left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 1, 2026

Uh oh!

BenjaminBossan commented Aug 20, 2026

Uh oh!

githubnemo left a comment

Choose a reason for hiding this comment

Uh oh!

githubnemo Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

peft-jambot commented Sep 10, 2026

Uh oh!

BenjaminBossan commented Sep 14, 2026

Uh oh!

peft-jambot commented Sep 14, 2026

Uh oh!

githubnemo left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants