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
+29 −16
Merged

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

Show file tree
Hide file tree
Changes from all commits
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
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
21 changes: 14 additions & 7 deletions gguf-py/gguf/scripts/gguf_editor_gui.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1517,27 +1517,34 @@ def save_file(self):
continue

# Apply changes if any
sub_type = None
if field.name in self.metadata_changes:
value_type, value = self.metadata_changes[field.name]
if value_type == GGUFValueType.ARRAY:
# Handle array values
element_type, array_values = value
writer.add_array(field.name, array_values)
else:
writer.add_key_value(field.name, value, value_type)
sub_type, value = value
else:
# Copy original value
value = field.contents()
if value is not None and field.types:
writer.add_key_value(field.name, value, field.types[0])
value_type = field.types[0]
if value_type == GGUFValueType.ARRAY:
sub_type = field.types[-1]

if value is not None:
writer.add_key_value(field.name, value, value_type, sub_type=sub_type)

# Add new metadata
for key, (value_type, value) in self.metadata_changes.items():
# Skip if the key already existed (we handled it above)
if self.reader.get_field(key) is not None:
continue

writer.add_key_value(key, value, value_type)
sub_type = None
if value_type == GGUFValueType.ARRAY:
# Handle array values
sub_type, value = value

writer.add_key_value(key, value, value_type, sub_type=sub_type)

# Add tensors (including data)
for tensor in self.reader.tensors:
Expand Down
7 changes: 5 additions & 2 deletions gguf-py/gguf/scripts/gguf_new_metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class MetadataDetails(NamedTuple):
type: gguf.GGUFValueType
value: Any
description: str = ''
sub_type: gguf.GGUFValueType | None = None


def get_field_data(reader: gguf.GGUFReader, key: str) -> Any:
Expand Down Expand Up @@ -57,7 +58,9 @@ def copy_with_new_metadata(reader: gguf.GGUFReader, writer: gguf.GGUFWriter, new
logger.debug(f'Removing {field.name}')
continue

old_val = MetadataDetails(field.types[0], field.contents())
val_type = field.types[0]
sub_type = field.types[-1] if val_type == gguf.GGUFValueType.ARRAY else None
old_val = MetadataDetails(val_type, field.contents(), sub_type=sub_type)
val = new_metadata.get(field.name, old_val)

if field.name in new_metadata:
Expand All @@ -67,7 +70,7 @@ def copy_with_new_metadata(reader: gguf.GGUFReader, writer: gguf.GGUFWriter, new
logger.debug(f'Copying {field.name}')

if val.value is not None:
writer.add_key_value(field.name, val.value, val.type)
writer.add_key_value(field.name, val.value, val.type, sub_type=sub_type if val.sub_type is None else val.sub_type)

if gguf.Keys.Tokenizer.CHAT_TEMPLATE in new_metadata:
logger.debug('Adding chat template(s)')
Expand Down
2 changes: 1 addition & 1 deletion gguf-py/pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tool.poetry]
name = "gguf"
version = "0.16.3"
version = "0.17.0"
description = "Read and write ML models in GGUF for GGML"
authors = ["GGML <ggml@ggml.ai>"]
packages = [
Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements-gguf_editor_gui.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
numpy~=1.26.4
PySide6~=6.9.0
gguf>=0.16.0
gguf>=0.17.0
0