Do we have to delete the PiSSA adapter after save_pissa_as_lora

[hiyouga]

System Info

peft v0.11.1 torch 2.3.0 linux

Who can help?

@BenjaminBossan

Information

• [ ] The official example scripts
• [X] My own modified scripts

Tasks

• [ ] An officially supported task in the examples folder
• [X] My own task or dataset (give details below)

Reproduction

I want to use the pissa adapter for other tasks after saving the model, but I cannot continue using the fine-tuned model.

https://github.com/huggingface/peft/blob/076561bbd32ce8cd649ba4ec05cbc592cf2dd1c6/src/peft/peft_model.py#L253-L276

Expected behavior

The PiSSA adapter can remain for further use.

[BenjaminBossan]

Okay, so if I understand correctly, you would like to skip the line self.delete_adapter(adapter_name) so that you can continue working with that adapter. I haven't thought about this use case. I can't quite remember if this was discussed with the PiSSA authors, but probably that line was included to avoid keeping two adapters in memory. But I see how this can be annoying.

Right now, the only way to really continue with this adapter is to reload the model, but I think we could make the change and if users want, they can delete the adapter themselves. Pinging @tokenizer-decode since they had some thoughts around this topic as well.

[hiyouga]

sure, a typical case is to evaluate the model after we save and convert the pissa adapter. Regarding the issue of maintaining two adapters in memory, should we delete the initial_adapter instead of the pissa adapter?

[tokenizer-decode]

I think the simplest approach that would work for you is just to comment out self.delete_adapter(adapter_name) line.

Regarding the issue of maintaining two adapters in memory, should we delete the initial_adapter instead of the pissa adapter?

If you want the use all of your memory, sure you can do that. I did not try that tbh, but since save_mutated_as_lora reloads the initial adapter, that should work just fine.

Regarding @BenjaminBossan

Right now, the only way to really continue with this adapter is to reload the model, but I think we could make the change and if users want, they can delete the adapter themselves.

Sure. But the only way for user to talk to save_mutated_as_lora is through save_pretrained. If this is necessary, you can just pass another option to save_pretrained such as delete_adapter. But you know my stance on adding other options to save_pretrained :)

Also if you are just experimenting, we'd be happy if you try out OLoRA. It's similar to PiSSA and that's why you see a unified approach there. I don't know how would it work out for your case btw. I don't have a comprehensive comparison between these methods. @hiyouga

[BenjaminBossan]

sure, a typical case is to evaluate the model after we save and convert the pissa adapter.

You could first evaluate and then save the model, but I would also prefer first saving and then evaluating, just in case the process crashes during evaluation.

Regarding the issue of maintaining two adapters in memory, should we delete the initial_adapter instead of the pissa adapter?

True. Honestly, I think I'd prefer not deleting any adapter and leaving this up to the user to decide. It does require more memory if the adapter is not deleted, but it doesn't change peak memory.

Sure. But the only way for user to talk to save_mutated_as_lora is through save_pretrained. If this is necessary, you can just pass another option to save_pretrained such as delete_adapter. But you know my stance on adding other options to save_pretrained :)

Yes, I agree that we should not add more options there.

[github-actions[bot]]

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]

Not stale, I hope that I can work on this soon.

[BenjaminBossan]

I created a PR to fix this: #1933. Hopefully I can a release soon and I thought it would be good to get this in before that, as technically, it's a breaking change.