Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Adapter/LoRA handling #1095

Open
5 tasks done
winglian opened this issue Jan 11, 2024 · 3 comments
Open
5 tasks done

Improve Adapter/LoRA handling #1095

winglian opened this issue Jan 11, 2024 · 3 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@winglian
Copy link
Collaborator

⚠️ Please check that this feature request hasn't been suggested before.

  • I searched previous Ideas in Discussions didn't find any similar feature requests.
  • I searched previous Issues didn't find any similar feature requests.

🔖 Feature description

there are a few use cases that aren't cleanly handled atm.

  • loading an existing adapter, then doing FFT over the merged model. We would need to load and merge the adapter and continue a regular FFT
  • loading an existing adapter and then training a new adapter over the merged model.

currently both of these can be worked around by simply manually merging the models beforehand, but it would be nice to handle these cases.

✔️ Solution

see above

❓ Alternatives

No response

📝 Additional Context

No response

Acknowledgements

  • My issue title is concise, descriptive, and in title casing.
  • I have searched the existing issues to make sure this feature has not been requested yet.
  • I have provided enough information for the maintainers to understand and evaluate this request.
@winglian winglian added enhancement New feature or request good first issue Good for newcomers labels Jan 11, 2024
@winglian winglian pinned this issue Jan 11, 2024
@winglian
Copy link
Collaborator Author

Additionally, we should simplify the lora/qlora w 8/4bit loading. Ideally we get rid of the adapter: qlora option as qlora is simply a specific subset of lora where all linear layers are targeted, and 4 bit quantization. I think we can simplify this to only lora and allow either 4 or 8bit to be set. And if a user selects qlora, then we warn about the specific cases where qlora applies - 4 bit and targeting all linear layers

@simhallq
Copy link
Contributor

simhallq commented Jan 14, 2024

I'll give this a go - just so I understand the first part correctly:

  • I should be able to do (full) finetuning with an existing adapter model as base_model arg in the training config. In that case the base model of the adapter should be merged with the adapter (i.e. using merge_and_unload) and then full finetune should be ran on the merged model.
  • Optionally full finetune can be swapped for new lora/qlora training run, adding new lora-layers/adapters to the merged model and then proceeding to train the new adapter weights only.

Is that right?

@winglian
Copy link
Collaborator Author

So in the first case, a user coukd add a lora model dir arg, but have an empty adapter. The model should simply just merge and unload the adapter into the base model only.

I'm not clear on what you are asking for the second case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants