Skip to content

Add support for GGUF machine learning model files #698

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

fegge
Copy link

@fegge fegge commented Jun 24, 2024

This PR adds support for the GGUF file format. GGUF is used to store machine learning models for executors based on the GGML machine learning library like llama.cpp and whisper.cpp developed by Meta.

Since there was no obvious location to add machine-learning model file formats, and since there are multiple other ML specific files formats that could be added in the future, I chose to create a new top-level ml directory and added the gguf.ksy file under that directory. I'm happy to put it somewhere else if someone has opinions on the location.

Example files for testing can be found under the models directory in the llama.cpp repository. (Small model files can be generated using the gguf utility in llama.cpp.)

model. It is also designed to be extensible, so that new information
can be added to models without breaking compatibility.
doc-ref:
- https://github.com/ggerganov/ggml/blob/master/docs/gguf.md
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess this should be changed to https://github.com/ggml-org/ggml/blob/master/docs/gguf.md ?

Copy link
Member

Choose a reason for hiding this comment

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

@armijnhemel:

I guess this should be changed to https://github.com/ggml-org/ggml/blob/master/docs/gguf.md ?

I agree, but please not master - always pin URLs to commit hashes to make them permanent. Pointing to the master branch doesn't even make sense semantically - this .ksy specification follows a particular current version of that Markdown file, not whatever version appears there in the future, which may be very different.

I should mention this rule in CONTRIBUTING.md and https://doc.kaitai.io/ksy_style_guide.html when I have some time...

type: u4
enum: ggml_type
- id: offset
type: u8
Copy link
Collaborator

Choose a reason for hiding this comment

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

This offset could be used to see and check if the data is really present. I took a GGUF file and cut it, was able to parse it, but it didn't do an extra sanity check to see if the offset was actually valid.

@generalmimon would you agree?

Copy link
Member

Choose a reason for hiding this comment

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

@armijnhemel:

This offset could be used to see and check if the data is really present.

Sure, why not. I'd prefer an "implicit check" by actually parsing the data at that offset using instances if possible - if the offset points after the end of file, parsing will fail. An "explicit check" using valid is also an option.

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