Combining adapters linearly with negative weights is broken

[orr-hirundo]

System Info

Hi,

I've noticed a bug when combining adapters with negative weights using _generalized_task_arithmetic_weighted_adapter. Negative weights are indeed handled (here) but then later (eg. in task_arithmetic and in other methods) they are applied to both the A and B LoRA deltas. If using a single adapter, this cancels out the negative weight; with multiple adapters, we can get unexpected behavior due to cross-terms of the (A_1+A_2)*(B_1+B_2) multiplication.

Who can help?

No response

Reproduction

I don't have a code snippet I can share, but calling add_weighted_adapter with a single adapter will have the same result whether using weights [1.0] or '[-1.0]`.

Expected behavior

Negative weights handled correctly - eg. sign only applied to A weights, or adapter negated upfront and then linear combination only happens with positive weights.

[github-actions]

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.

[BenjaminBossan]

Sorry, I somehow missed this issue, will take a look next week.

[github-actions]

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.

[BenjaminBossan]

Hi @orr-hirundo, sorry for the late reply. I was waiting for movement on #3013 but it seems the author is MIA. So I took a closer look myself and I came to the conclusion that the combination_type="linear" is just fundamentally plagued by the cross terms, whether we have negative weights or not. I would recommend to go with "svd" (requires more computation upfront) or "cat" (cheap to create but higher runtime compute and memory) instead. There were still bugs in the implementation (most notably with "svd") and the testing, but I'm addressing them in #3111.

[orr-hirundo]

Thanks, I've been sticking with cat anyway since it does what I expected linear would do in the first place.

[github-actions]

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.