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

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

Merged
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
@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.

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

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 ggerganov:master Aug 23, 2024
9 checks passed
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 15, 2024
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python python script changes testing Everything test related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Converted HF LoRA adapter on Llama 3.1 not loading.
3 participants