Skip to content
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: missing split.no metadata #604

Closed
ngxson opened this issue Apr 3, 2024 · 6 comments
Closed

GGUF: missing split.no metadata #604

ngxson opened this issue Apr 3, 2024 · 6 comments
Labels
gguf good first issue Good for newcomers help wanted Extra attention is needed

Comments

@ngxson
Copy link
Member

ngxson commented Apr 3, 2024

Related to: ggerganov/llama.cpp#6343 (comment)

Explanation: We recently introduced gguf-split tool to llama.cpp, which allows user to split the model into smaller shards. Each shard has 3 metadata to know its info:

  • split.count: Total number of splits
  • split.no: The number of the current split
  • split.tensors.count: Total number of tensors of the original model (= sum of tensors of all splits)

The split.no is however missing when viewing from GGUF viewer on huggingface. It is still visible when inspecting using gguf-py

image

This can be reproduce using a smaller model: https://huggingface.co/ngxson/tinyllama_split_test/tree/main?show_tensors=stories15M-q8_0-00001-of-00003.gguf

image

Here is the command that I used to split the model:

./gguf-split --split-max-size 10M ~/Downloads/stories15M-q8_0.gguf ~/Downloads/stories15M-q8_0

I'd be happy to help you guys with this. Feel free to let me know if you need more info.

@julien-c julien-c added good first issue Good for newcomers help wanted Extra attention is needed gguf labels Apr 3, 2024
@mishig25
Copy link
Collaborator

mishig25 commented Apr 3, 2024

@ngxson you can run the gguf js parser in any node/js env without needing hf frontend, as described in readme https://github.com/huggingface/huggingface.js/blob/main/packages%2Fgguf%2FREADME.md

If you can produce a snippet to reproduce the bug or even better a fix for the gguf js parser, that would be great !

@phymbert
Copy link

phymbert commented Apr 9, 2024

@julien-c The bug seems easy to fix: do not remove metadata with 0 as value :)
Please include all metadata regardless their value.

@mishig25
Copy link
Collaborator

mishig25 commented Apr 9, 2024

@phymbert thanks a lot for debugging. There was no problem on the js/parser side (i.e. metadata had split.no: 0). However, frontend was treating 0 as falsy, therefore not rendering. Hence, I've submitted a PR that fixes the issue on the frontend

Screenshot 2024-04-09 at 11 46 46

@phymbert
Copy link

phymbert commented Apr 9, 2024

Cool! thanks @mishig25 for the explanations

@mishig25 mishig25 closed this as completed Apr 9, 2024
@mishig25 mishig25 reopened this Apr 9, 2024
@mishig25
Copy link
Collaborator

mishig25 commented Apr 9, 2024

Keeping the issue open until the UI fix is deployed (the fix is already merged)

@mishig25
Copy link
Collaborator

mishig25 commented Apr 9, 2024

The fix is deployed now. You can see results https://huggingface.co/ngxson/tinyllama_split_test/tree/main?show_tensors=stories15M-q8_0-00001-of-00003.gguf

@mishig25 mishig25 closed this as completed Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gguf good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants