-
Notifications
You must be signed in to change notification settings - Fork 469
[GGUF] typed metadata #1649
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
[GGUF] typed metadata #1649
Conversation
…ieve metadata with type information.
|
||
// Check if scores array is properly handled | ||
if (typedMetadata["tokenizer.ggml.scores"]) { | ||
expect(typedMetadata["tokenizer.ggml.scores"].type).toEqual(GGUFValueType.ARRAY); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering, should we now separate GGUFValueType.ARRAY
into GGUFValueType.ARRAY_INT32
, GGUFValueType.ARRAY_STRING
, etc ?
It will come in handy when we want to distinguish among array of uint, int or float
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that would be diverging from the gguf spec, no ?
https://github.com/ggml-org/ggml/blob/master/docs/gguf.md?plain=1#L191
there is no ARRAY_STRING or ARRAY_INT32 in enum gguf_metadata_value_type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pushed 0ba56b1
if type is array, there will be property subType
as well
// "tokenizer.ggml.tokens": { value: ["<unk>", "<s>", "</s>", ...], type: GGUFValueType.ARRAY, subType: GGUFValueType.STRING },
// "tokenizer.ggml.scores": { value: [0.0, -1000.0, -1000.0, ...], type: GGUFValueType.ARRAY, subType: GGUFValueType.FLOAT32 },
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would keep things simple personally but 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes subType
would work too!
This is necessary because we need to reconstruct the array with the original type. Otherwise GGUF will fail to load if the element type mismatched (which can be the case for float/uint/int)
Description
Enhance GGUF functionality by adding typedMetadata support
This update introduces typedMetadata to the gguf function, allowing users to request structured metadata alongside the standard output. The implementation includes checks for both V1 and V2 file formats, ensuring compatibility and consistency in metadata retrieval. Additionally, tests have been added to validate the new functionality and ensure that metadata values align correctly between standard and typed formats.
Usage