ENH: Enable OFT adapter for mixed adapter models #1204
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR makes it possible to use the newly added OFT adapter in mixed adapter type models, similar to LoRA, LoHa, etc. Notes Adding the integration was pretty straightforward, which is a good sign. The difficult part was actually about the tests. This stems from the fact that OFT is (if my understanding is correct) never commutative. What I mean is that even if the adapters are applied to the last layer of a model, it makes a difference whether we apply, say, first LoRA, then OFT vs first OFT, then LoRA. This is different for the other adapters that were added so far for mixed models, as they basically do: - Xa = X + dXa - Xab = Xa + dXb = X + dXa + dXb = X + dXb + dXa = Xb + dXa = Xba IIUC, this is not true for OFT, so when OFT is used, I had to ensure that no test was applied that (implicitly) assumes commutativity. Furthermore, I had to increase the model size, see this comment: huggingface#1160 (comment)
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. |
No point in including "Config".
On CPU, apparently higher tolerance is required than on GPU.
|
As far as my understanding of OFT goes your assumption about commutativity are correct |
|
@BenjaminBossan It's correct. |
younesbelkada
approved these changes
Dec 4, 2023
|
Thanks @lukaskuhn-lku and @okotaku for your input, and @younesbelkada for the review. |
TaoSunVoyage
pushed a commit
to TaoSunVoyage/peft
that referenced
this pull request
Dec 14, 2023
This PR makes it possible to use the newly added OFT adapter in mixed adapter type models, similar to LoRA, LoHa, etc. Notes Adding the integration was pretty straightforward, which is a good sign. The difficult part was actually about the tests. This stems from the fact that OFT is (if my understanding is correct) never commutative. What I mean is that even if the adapters are applied to the last layer of a model, it makes a difference whether we apply, say, first LoRA, then OFT vs first OFT, then LoRA. This is different for the other adapters that were added so far for mixed models, as they basically do: - Xa = X + dXa - Xab = Xa + dXb = X + dXa + dXb = X + dXb + dXa = Xb + dXa = Xba This is not true for OFT, so when OFT is used, I had to ensure that no test was applied that (implicitly) assumes commutativity. Furthermore, I had to increase the model size, see this comment: huggingface#1160 (comment)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes it possible to use the newly added OFT adapter in mixed adapter type models, similar to LoRA, LoHa, etc.
Notes
Adding the integration was pretty straightforward, which is a good sign.
The difficult part was actually about the tests. This stems from the fact that OFT is (if my understanding is correct) never commutative. What I mean is that even if the adapters are applied to the last layer of a model, it makes a difference whether we apply, say, first LoRA, then OFT vs first OFT, then LoRA.
This is different for the other adapters that were added so far for mixed models, as they basically do:
Xa = X + dXaXab = Xa + dXb = X + dXa + dXb = X + dXb + dXa = Xb + dXa = XbaIIUC, this is not true for OFT, so when OFT is used, I had to ensure that no test was applied that (implicitly) assumes commutativity. Ping @okotaku and @lukaskuhn-lku is my understanding correct?
Furthermore, I had to increase the model size, see this comment:
#1160 (comment)