Skip to content
This repository was archived by the owner on Jun 24, 2024. It is now read-only.

Support latest (breaking) changes to GGML file format#61

Merged
setzer22 merged 5 commits intorustformers:mainfrom
RoyVorster:ggml-fix
Mar 23, 2023
Merged

Support latest (breaking) changes to GGML file format#61
setzer22 merged 5 commits intorustformers:mainfrom
RoyVorster:ggml-fix

Conversation

@RoyVorster
Copy link
Copy Markdown
Contributor

@RoyVorster RoyVorster commented Mar 23, 2023

Latest ggml models have:

  • Different magic
  • A format version
  • Scores for tokens (currently parsed but unused)

This PR maintains support for older ('legacy') models
A model downloaded and converted on latest master in llama.cpp now works with llama-rs but the output seems qualitatively worse. Haven't played around much with llama-rs though so it's hard to say whether this is a regression.

RoyVorster
RoyVorster commented Mar 23, 2023
View reviewed changes
Comment thread llama-rs/src/lib.rs Outdated
@mguinhos mguinhos mentioned this pull request Mar 23, 2023
Closed
@philpax philpax mentioned this pull request Mar 23, 2023
Closed
@setzer22
Copy link
Copy Markdown
Collaborator

setzer22 commented Mar 23, 2023 •
edited
Loading

Hi! Thanks a lot for the PR 😄

A model downloaded and converted on latest master in llama.cpp now works with llama-rs but the output seems qualitatively worse.

That's really strange. IIRC the only changes to the format are that now vocab scores are embedded in the model. Weights should be exactly the same.

Can you make a test with --seed to ensure results are the same between this and the main branch? Otherwise this may be a hint at a bug somewhere 🤔

setzer22
setzer22 approved these changes Mar 23, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@setzer22 setzer22 left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks again for the changes. Can you sort out the merge conflict and the formatting / clippy issues from CI?

Once that's taken care of, we're ready to merge 😄

Comment thread llama-rs/src/lib.rs Outdated
@RoyVorster
Copy link
Copy Markdown
Contributor Author

RoyVorster commented Mar 23, 2023

Yup, will doublecheck output vs. main branch on an older model to ensure all is okay.

@RoyVorster
Make this work with current format ggml models
dccbbc4 
Breaking changes in ggml:
- Scores added in vocabulary
- Format version added
- Magic updated
@RoyVorster
Add some semblance of support for legacy models
42f26ea
@RoyVorster
Cleaner handling of different types in byte reader
2f57987 
- Generalize u32, i32 and f32 reading without a boilerplate-y trait
@RoyVorster
Fix formatting
d59f981
@setzer22 setzer22 mentioned this pull request Mar 23, 2023
Merged
@RoyVorster
Copy link
Copy Markdown
Contributor Author

RoyVorster commented Mar 23, 2023

Yeah looks good to me. @setzer22 can you approve the workflow?

@setzer22 setzer22 merged commit a1122dd into rustformers:main Mar 23, 2023
@setzer22
Copy link
Copy Markdown
Collaborator

setzer22 commented Mar 23, 2023

Merged! Took care of some minor clippy lints myself.

Thanks again :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@setzer22 setzer22 setzer22 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@RoyVorster @setzer22