8000 gguf-py : add support for sub_type (in arrays) in GGUFWriter add_key_value method by CISC · Pull Request #13561 · ggml-org/llama.cpp · GitHub
[go: up one dir, main page]
Skip to content

gguf-py : add support for sub_type (in arrays) in GGUFWriter add_key_value method #13561

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
May 29, 2025
Merged

gguf-py : add support for sub_type (in arrays) in GGUFWriter add_key_value method #13561

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2821bfc
add support for sub_type to add_key_value method
CISC May 15, 2025
64d54b2
specify sub_type when passing through metadata
CISC May 15, 2025
7aefca3
bump version
CISC May 15, 2025
652afd0
bump requirements
CISC May 15, 2025
dbc1a49
avoid explicitly setting blank description [no ci]
CISC May 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
add support for sub_type to add_key_value method
  • Loading branch information
@CISC
CISC authored May 15, 2025
commit 2821bfcfa01a3924b37f54273eecc6d820d4d12c
13 changes: 8 additions & 5 deletions gguf-py/gguf/gguf_writer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class TensorInfo:
class GGUFValue:
value: Any
type: GGUFValueType
sub_type: GGUFValueType | None = None


class WriterState(Enum):
Expand Down Expand Up @@ -238,7 +239,7 @@ def write_kv_data_to_file(self) -> None:

for key, val in kv_data.items():
kv_bytes += self._pack_val(key, GGUFValueType.STRING, add_vtype=False)
kv_bytes += self._pack_val(val.value, val.type, add_vtype=True)
kv_bytes += self._pack_val(val.value, val.type, add_vtype=True, sub_type=val.sub_type)

fout.write(kv_bytes)

Expand Down Expand Up @@ -268,11 +269,11 @@ def write_ti_data_to_file(self) -> None:
fout.flush()
self.state = WriterState.TI_DATA

def add_key_value(self, key: str, val: Any, vtype: GGUFValueType) -> None:
def add_key_value(self, key: str, val: Any, vtype: GGUFValueType, sub_type: GGUFValueType | None = None) -> None:
if any(key in kv_data for kv_data in self.kv_data):
raise ValueError(f'Duplicated key name {key!r}')

self.kv_data[0][key] = GGUFValue(value=val, type=vtype)
self.kv_data[0][key] = GGUFValue(value=val, type=vtype, sub_type=sub_type)

def add_uint8(self, key: str, val: int) -> None:
self.add_key_value(key,val, GGUFValueType.UINT8)
Expand Down Expand Up @@ -993,7 +994,7 @@ def _pack(self, fmt: str, value: Any, skip_pack_prefix: bool = False) -> bytes:
pack_prefix = '<' if self.endianess == GGUFEndian.LITTLE else '>'
return struct.pack(f'{pack_prefix}{fmt}', value)

def _pack_val(self, val: Any, vtype: GGUFValueType, add_vtype: bool) -> bytes:
def _pack_val(self, val: Any, vtype: GGUFValueType, add_vtype: bool, sub_type: GGUFValueType | None = None) -> bytes:
kv_data = bytearray()

if add_vtype:
Expand All @@ -1014,7 +1015,9 @@ def _pack_val(self, val: Any, vtype: GGUFValueType, add_vtype: bool) -> bytes:
if len(val) == 0:
raise ValueError("Invalid GGUF metadata array. Empty array")

if isinstance(val, bytes):
if sub_type is not None:
ltype = sub_type
Comment on lines +1018 to +1019
Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding some dynamic type-checking here could be useful (to check if all the elements of the array have the same type and are compatible with the given sub_type), but a traceback from this function isn't that useful because it's called when packing, not when adding the value to the writer.

Not sure what would be best here. I guess it's fine like this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I don't think it's very useful, it will mostly likely be used for passing existing GGUF arrays anyway.

We can always revisit if it becomes an issue.

elif isinstance(val, bytes):
ltype = GGUFValueType.UINT8
else:
ltype = GGUFValueType.get_type(val[0])
Expand Down
0