Skip to content

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