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

Add back meta-device assign=True loading in merge_lora #1250

Merged
merged 8 commits into from
May 7, 2024

Conversation

carmocca
Copy link
Contributor

@carmocca carmocca commented Apr 4, 2024

This reverts commit d8dc97e.

@carmocca carmocca self-assigned this Apr 4, 2024
@carmocca carmocca changed the title Revert "Fix dtype mismatch when merging LoRA checkpoints (#1246)" Ensure pretrained and lora weights have the same dtype in merge_lora Apr 4, 2024
@carmocca carmocca marked this pull request as ready for review April 4, 2024 17:14
@carmocca carmocca changed the title Ensure pretrained and lora weights have the same dtype in merge_lora Add back meta-device assign=True loading in merge_lora Apr 30, 2024
@carmocca carmocca requested a review from awaelchli April 30, 2024 01:01
@carmocca
Copy link
Contributor Author

carmocca commented May 6, 2024

Blocked by #1385, #1378

@carmocca
Copy link
Contributor Author

carmocca commented May 6, 2024

@awaelchli Do you want to review this again or am I good to merge this?

@awaelchli
Copy link
Contributor

@carmocca I lost a bit track of the intermediate changes that happened between my commit and this revert. But I'm good with merging if we're confident it was fixed properly. Ultimately, what needs to work is being able to merge a base model in float16 and a lora checkpoint in bfloat16, or vice versa. Which I think is covered in this test now: https://github.com/Lightning-AI/litgpt/pull/1251/files

@carmocca carmocca merged commit c0d1dd0 into main May 7, 2024
9 checks passed
@carmocca carmocca deleted the carmocca/revert-stuff branch May 7, 2024 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants