gguf : deduplicate

[ggerganov]

Following things work:

• mmap
• mlock
• 7B model:
  • quantize
  • main MacOS CPU
  • main MacOS Metal
  • main Ubuntu CPU
  • main Ubuntu CUDA
  • perplexity MacOS CPU
  • perplexity MacOS Metal
  • perplexity Ubuntu CPU
  • perplexity Ubuntu CUDA
  • simple
• llama load/save session
• non-7B models (not tested)

[ggerganov]

Overall, it is looking good - need to test and fix CUDA.

The only problem I notice is that there are no more new-lines in the generated text.
Maybe something wrong after the tokenizer fixes.
cc @goerch - any ideas?

[monatis]

@ggerganov Btw, I removed the inner part of llama_load_session_file_internal() and its save counterpart, thinking of migrating them to GGUF format later on. So these need to be copied from old llama.cpp again if we don't want to do so.

My idea was to use GGUF for all serialization / deserialization ops, including session and LoRA files, but we may choose to keep things simple.

[klosax]

Overall, it is looking good - need to test and fix CUDA

Compiling with cublas seems to work fine. Tested main perplexity quantize on openllama-3b-v2

[klosax]

Still got this compiler warning:

In file included from /usr/include/string.h:495,
                 from /usr/include/c++/9/cstring:42,
                 from llama.cpp:72:
In function ‘char* strncpy(char*, const char*, size_t)’,
    inlined from ‘int llama_token_to_str_with_model(const llama_model*, llama_token, char*, int)’ at llama.cpp:4793:20:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:34: warning: ‘char* __builtin_strncpy(char*, const char*, long unsigned int)’ output truncated before terminating nul copying 3 bytes from a string of the same length [-Wstringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[ggerganov]

@monatis Yup, I will fix these soon.

@klosax Cool

Regarding the "new-line" issue - I can see the new-line token (13) being sampled, but it is detokenized to empty string '' instead of new-line by the llama_token_to_str() function. We should dig into this and fix it

[klosax]

Maybe the warning above is the problem?

[klosax]

Line feed working now, I guess it was a typo + instead of -

[ggerganov]

Perfect - the tokenizer test don't work because they need new vocabs:

 $ ▶ ./bin/test-tokenizer-0 ../models/ggml-vocab-llama.bin 
main : reading vocab from: '../models/ggml-vocab-llama.bin'
gguf_init_from_file: invalid magic number 67676a74
Segmentation fault (core dumped)

We need to generate new vocab files.

[klosax]

Maybe better to use a model file instead of using a separate vocab? And have information that the tests only works with the original LLaMA models.

[klosax]

Later we will need tokenizer tests for other models like mpt falcon gptneox..

[ggerganov]

We need small vocab files inside the repository in order to run the CI

[klosax]

Ok. The vocab files are model files without the tensors, right?

[ggerganov]

Yes, just the meta data: key-value pairs + tensor info. No tensor data

[klosax]

Should not have tensor_count = 0 and include tensor info? Maybe skip tensor info?

[ggerganov]

Should not have tensor_count = 0 and include tensor info? Maybe skip tensor info?

Both ways works - whichever is simpler

[monatis]

Setting tensor count to 0 perfectly works without breaking file format --I previously used it for testing.

We can define a flag in conversion script metadata_only = False and setting it to True can output metadata-only file without tensors for that model.

[ggerganov]

There is already --vocab-only arg in convert.py:

https://github.com/ggerganov/llama.cpp/blob/5b94b14d5dd59e14f2ab443a79413c445726c98f/convert.py#L1305

I consider the other convert scripts that we currently use just temporary until we update convert.py to work with gguf.py, so not sure it's worth adding more features to them

Edit: this is regarding the LLaMA models. For other models, we should likely have separate convert scripts

[klosax]

Edit: this is regarding the LLaMA models. For other models, we should likely have separate convert scripts

I that case i think the lazy tensor reader from convert.py should be separated so they can be used by the other scripts.

[klosax]

LLaMA v1 13b main and perplexity works

[klosax]

Tested q4_0 70b model in main but the output is not good and perplexity returns -inf ..

[klosax]

LLaMA v1 33b works in main and perplexity

[klosax]

OpenLLaMA v2 3b and 7b works main perplexity

[klosax]

LLaMA v1 7b also works.

[ggerganov]

Weird that 70B does not work - I can't test as I don't have HF 70B. Just the original one

Will merge like this and fix the convert script. When ready, will test 70B and try to resolve

[Green-Sky]

Weird that 70B does not work - I can't test as I don't have HF 70B. Just the original one

Will merge like this and fix the convert script. When ready, will test 70B and try to resolve

iirc there was a fix on master after gguf branched. (cant find it rn so might be bad memory)

[klosax]

iirc there was a fix on master after gguf branched.

If you was thinking of this #2427 it is already included.
I cant fully test the 70b model on my system.

[Green-Sky]

If you was thinking of this #2427 it is already included.
I cant fully test the 70b model on my system.

hm yea, i think i meant that. never mind then.

[goerch]

Just tried to the branch. Converting with

python.exe convert-llama-h5-to-gguf.py models\llama-7B-hf 1

looks fine:

output.weight, shape 2, torch.float16 --> float16
gguf: model successfully exported to 'models\llama-7B-hf/ggml-model-f16.gguf'

The resulting file gguf file is around 13 GB big. When starting inference I get

main: build = 1168 (d864596)
main: seed  = 1692248927
gguf_init_from_file: invalid magic number 00000000

This happens with clang-cl on Windows.

[klosax]

gguf_init_from_file: invalid magic number 00000000

I got this when accidentally tried to open a directory instead of a model file.

[monatis]

I got this when accidentally tried to open a directory instead of a model

Me too, and reproduceable.

$ ./main -m ~                                                       

main: build = 1157 (5ec1893)                                                                                            
main: seed  = 1692257530                                                                                                
gguf_init_from_file: invalid magic number 00000000                                                                      
Segmentation fault                                                                                                      

[klosax]

gguf: model successfully exported to 'models\llama-7B-hf/ggml-model-f16.gguf'

There is both backslashes and slashes in this path, dont know if you are using this to load?

[ggerganov]

I don't have Windows to test

I'm a bit confused - does the wrong magic of 0000000 occur for directories, or does it occur with a converted F16 model file?

[klosax]

I get the error when trying to open a directory instead of a model file. On ubuntu.

[goerch]

My mistake was trying to open an non existing file. Otherwise things work fine now.

[monatis]

does the wrong magic of 0000000 occur for directories, or does it occur with a converted F16 model?

To my end, I couldn't reproduce with a GGUF file, but passing a directory path leads to wrong magic on WSL, regular Windows and regular Ubuntu.

[monatis]

Maybe we can make that error message a little bit verbose with "Are you sure that this path holds a GGUF file?"

[klosax]

Better " Can not open an directory. Should be a .gguf model file."

[ggerganov]

@goerch

I'm running the test-tokenizer-0 test using the new F16 GGUF model, and it is currently failing.
We made the following change recently:

from:

static uint8_t llama_byte_to_char(const llama_vocab & vocab, uint8_t byte) {
    if (llama_vocab_type(vocab) == "spm") {
        return byte + 3;
    }

to:

        return byte - 3;

This was needed because the new-line wasn't printing correctly.
However, with this change, the test-tokenizer-0 is now failing.

If I revert it, it no longer fails, but the de-tokenized strings do not match the original strings.
For example:

main : ' ' tokenized to ' ▁'
main : ' Hello World' tokenized to '  Hello World'
main : ' Hello World!' tokenized to '  Hello World!'
main : ' Hello world' tokenized to '  Hello world'
main : ' this is 🦙.cpp' tokenized to '  this is .cpp'
main : 'Hello World' tokenized to ' Hello World'
main : 'Hello world' tokenized to ' Hello world'
main : 'w048 7tuijk dsdfhu' tokenized to ' w048 7tuijk dsdfhu'
main : 'нещо на Български' tokenized to ' нещо на Български'
main : 'កាន់តែពិសេសអាចខលចេញ' tokenized to ' កាន祑礕祈礜礽ស祇ស礨ា礋礇礡礋祇礏'
main : '🚀 (normal) 😶‍🌫️ (multiple emojis concatenated) ✅ (only emoji that has its own token)' tokenized to '  (normal) ‍️ (multiple emojis concatenated) ✅ (only emoji that has its own token)'

I suspect 2 things:

• either I introduced a bug when merging the tokenizer fixes PR
• or we are not exporting the vocab properly using the new convert scripts

Will be looking into this now - any help would be appreciated

[ggerganov]

I think I fixed it:

7b6ae89

We were incorrectly using llama_byte_to_char() when tokenizing a string.
To fix it, I added llama_char_to_byte() and use it instead

[cebtenzzre]

There's a missing check for ctx_gguf == NULL here, which is why "invalid magic number" leads to a segfault.

[cebtenzzre]

And checking ferror(file) if gguf_fread_el returns false here should make it more obvious if the user passes a directory instead of a file:
https://github.com/ggerganov/llama.cpp/blob/795ec7070cba4356112836bd0de3a718cb04e8bf/ggml.c#L18717-L18721

[klosax]

I think it is better if you review here instead #2398