Skip to content

lora : fix llama conversion script with model having ROPE_FREQS #9117

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 23, 2024
Merged

lora : fix llama conversion script with model having ROPE_FREQS #9117

merged 1 commit into from
Aug 23, 2024

Conversation

ngxson
Copy link
Collaborator

@ngxson ngxson commented Aug 21, 2024

Resolve #9114

@ngxson ngxson requested a review from compilade August 21, 2024 12:55
@ngxson ngxson mentioned this pull request Aug 21, 2024
Closed
@github-actions github-actions bot added testing Everything test related python python script changes labels Aug 21, 2024
@Ujjawal-K-Panchal
Copy link
Contributor

This fixes the issue #9114 I raised. Please look at the final output log here.

compilade
compilade approved these changes Aug 22, 2024
View reviewed changes
convert_hf_to_gguf.py
Comment on lines +1598 to +1599
if not self.is_lora:
self.gguf_writer.add_tensor(self.format_tensor_name(gguf.MODEL_TENSOR.ROPE_FREQS), np.array(rope_factors, dtype=np.float32))
Copy link
Collaborator

@compilade compilade Aug 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Phi-3, vocab-only conversion is also affected by these rope_freqs tensors, because this is in set_gguf_parameters. (which makes vocab-only Phi-3-128k models produce invalid GGUF files (this is already a problem on master))

A more general solution to both LoRA and vocab-only conversions should be possible.

Maybe some kind of self.generate_extra_tensors() which would be called by self.prepare_tensors() before it calls self.get_tensors(). And LoraModel could simply override generate_extra_tensors() to a no-op (and vocab-only conversion does not call prepare_tensors). It can be done in a follow-up PR, though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I'll merge this now and will let you refactor this further in a follow-up PR.
Thank you for the help!

@ngxson ngxson merged commit 3ba780e into ggml-org:master Aug 23, 2024
9 checks passed
@compilade compilade mentioned this pull request Sep 10, 2024
Merged
12 tasks
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 15, 2024
@ngxson @arthw
lora : fix llama conversion script with ROPE_FREQS (ggml-org#9117)
f4e85a1
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 18, 2024
@ngxson @arthw
lora : fix llama conversion script with ROPE_FREQS (ggml-org#9117)
d19ac68
Nexesenex pushed a commit to Nexesenex/croco.cpp that referenced this pull request Feb 25, 2025
@ngxson @Nexesenex
lora : fix llama conversion script with ROPE_FREQS (ggml-org#9117)
5aa07dc
Nexesenex pushed a commit to Nexesenex/croco.cpp that referenced this pull request Feb 25, 2025
@ngxson @Nexesenex
lora : fix llama conversion script with ROPE_FREQS (ggml-org#9117)
5019bc0
@saood06 saood06 mentioned this pull request May 9, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compilade compilade compilade approved these changes

Assignees
No one assigned
Labels
python python script changes testing Everything test related
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug: Converted HF LoRA adapter on Llama 3.1 not loading.
3 participants
@ngxson @Ujjawal-K-Panchal @compilade