[from_pretrained] Simpler code for peft#25726
Conversation
Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
|
The documentation is not available anymore as the PR was closed or merged. |
…to model-utils-nits
…ansformers into model-utils-nits
There was a problem hiding this comment.
Thanks a lot @ArthurZucker for attempting to refactor that part, sadly all PEFT tests are failing with your changes, can you please make sure they pass before merging?
RUN_SLOW=1 pytest tests/peft_integration/test_peft_integration.pyAlso make sure to rebase with main in order to run the tests
I suspect the reason is because you need to override the pretrained_model_name_or_path instance with _adapter_model_path and store the current adapter id in a new variable adapter_model_id in the previous changes
|
Thanks! Can confirm that the tests are now green! @younesbelkada some are really fast do you not want to remove the |
…to model-utils-nits
younesbelkada
left a comment
There was a problem hiding this comment.
Thanks! All of them should be using tiny models so they are fast, I don't know if we want to add PEFT as a dependency to our CIs so maybe let's keep it as it is for now, what do you think?
|
@ArthurZucker sorry the tests are still failing, probably something wrong happened during the merge, can you double check? 🙏 I can also have a look if you want Edit: just made #25733 |
* refactor complicated from pretrained for peft * nits * more nits * Update src/transformers/modeling_utils.py Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com> * make tests happy * fixup after merge --------- Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
What does this PR do?
Simplifies the logic when loading a PEFT model: the
_adapter_model_pathis used instead ofmaybe_has_adapter_model_pathandhas_adapter_config