FIX Raise an error when performing mixed adapter inference and passing non-existing adapter names #2090
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
PEFT allows mixed batch adapter inference, i.e. when predicting, the same batch can use different adapters by passing the adapter_names argument. However, when users pass an adapter name that does not correspond to any of the existing adapters, these samples are currently being ignored (i.e. just the base model output is used). This is unexpected and can easily lead to errors, e.g. when users mistype the name of an adapter. This PR fixes this issue by checking all the existing adapter names first and comparing them to the adapter_names that the user passed. If there are unexpected entries, an error is raised. Due to this fix, an error in the test test_mixed_adapter_batches_lora_merged_raises was discovered and promptly fixed.
|
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. |
|
ping @SunMarc Could I get a review please, it should hopefully not take long. |
SunMarc
approved these changes
Oct 9, 2024
|
No worries, that happens sometimes. Thanks a lot for the review. |
BenjaminBossan
deleted the
fix-raise-error-mixed-batch-inference-with-non-existing-adapters
branch
BenjaminBossan
added a commit
to BenjaminBossan/peft
that referenced
this pull request
Oct 22, 2024
PEFT allows mixed batch adapter inference, i.e. when predicting, the same batch can use different adapters by passing the adapter_names argument. However, when users pass an adapter name that does not correspond to any of the existing adapters, these samples are currently being ignored (i.e. just the base model output is used). This is unexpected and can easily lead to errors, e.g. when users mistype the name of an adapter. This PR fixes this issue by checking all the existing adapter names first and comparing them to the adapter_names that the user passed. If there are unexpected entries, an error is raised. Due to this fix, an error in the test test_mixed_adapter_batches_lora_merged_raises was discovered and promptly fixed.
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.
PEFT allows mixed batch adapter inference, i.e. when predicting, the same batch can use different adapters by passing the
adapter_namesargument. However, when users pass an adapter name that does not correspond to any of the existing adapters, these samples are currently being ignored (i.e. just the base model output is used). This is unexpected and can easily lead to errors, e.g. when users mistype the name of an adapter.This PR fixes this issue by checking all the existing adapter names first and comparing them to the
adapter_namesthat the user passed. If there are unexpected entries, an error is raised.Due to this fix, an error in the test
test_mixed_adapter_batches_lora_merged_raiseswas discovered and promptly fixed.