[petersid2022]

Overview

• when we select the COMMON_SPECULATIVE_TYPE_NGRAM_CACHE speculative implementation we create a new common_speculative_state_ngram_cache state using create_state_ngram_cache, where we instantiate the new state by specifying various parameters (e.g, save_static and save_dynamic) by hardcoding them.

• remove those options and only depend on the existence of lookup_cache_dynamic and lookup_cache_static (both user provided) for enabling the saving and updating the dynamic cache into a file.

• on object destruction check if the above functionality is supported (i.e. if the lookup_cache_dynamic path is provided by the user) then merge all of the ngram_cache_context from each sequence info and save the final common_ngram_cache to a file using common_ngram_cache_save.

• the class now saves the file on destruction, so it's built directly in a unique_ptr to avoid the temporary copy that would trigger an extra save

• if the file does not exist, starts with an empty cache that is written on exit. if the file exists but cannot be read (permissions, path is a directory, etc.), aborts instead of overwriting it with only the current session's n-grams.

• goal: --lookup-cache-dynamic is never written on master, and a path that doesn't exist aborts.

• fix draft_one method not updating the ngram cache history correctly

• common_ngram_cache_update received only the new tokens with none of the tokens before them causing ngrams ending at the new token to not being formed, leading the context cache to only ever hold the prompt

• fix: pass the full history to common_ngram_cache_update with nnew to the number of new tokens the way llama-lookup does and also reset cache_size to 0 on each new request

Additional information

Add self-speculative decoding (no draft model required) PR: #18471

Requirements

• I have read and agree with the contributing guidelines
• AI usage disclosure: YES (Claude Opus 5.5) reviewing my code and helping me pinpoint and understand the draft_one bug

[petersid2022]

@ggerganov kind reminder for a code review of this PR, that simply saves the dynamic ngram cache file (when a path is provided) to the filesystem on object destruction. Thanks in advance!
BR,
Peter Sideris

[petersid2022]

after testing on a RTX A6000 GPU the new implementation of the PR like so:

$ llama-server -m ~/models/qwen2.5-0.5b-instruct-q8_0.gguf -ngl 99 -np 1 -c 4096 --port 18555 --spec-type ngram-cache -lcd /tmp/pr22055-dyn.bin
$ curl -s localhost:18555/completion -d '{"prompt":"Write a detailed explanation of how a hash table works, including collision handling.","n_predict":400,"temperature":0,"cache_prompt":false}' | jq -c '{n: .timings.predicted_n, tps: .timings.predicted_per_second, draft_n: .timings.draft_n, acc: .timings.draft_n_accepted}'

on various test cases

• not existing dynamic cache file (/tmp/pr22055-dyn.bin) on llama-server start which should make the server warn the user, use an empty cache and finally create the file on shutdown (when the destructor runs)
• reusing the same server command again and again, causing the file size to grow as the cache builds up
• chmod 000 on the dynamic cache file which should cause the llama-server to abort and isn't overwritten
• not existing static cache file should cause the llama-server to abort

Upon doing so, I managed to find a bug that was on the master branch after seeing very low draft acceptance, on the draft_one method of the common_speculative_impl_ngram_cache, the ngram_cache never learns generated tokens, due to common_ngram_cache_update getting passed only the new tokens (n-gram ending at a new token needs the tokens before it, and those weren't passed), so I fixed it by following the implementation of llama-lookup

[Green-Sky]

Tested and works.

Unrelated: Due to missing control, you cant limit the lenght of the prediction and maybe other things, resulting in sub-par speculation quality and effectively slowing gen down...

I spec common_specu: statistics        ngram-mod: #calls(b,g,a) =    2    968     11, #gen drafts =     11, #acc drafts =     9, #gen tokens =     44, #acc tokens =    33, #mean acc len = 4.00, #acc rate/pos = (0.818, 0.727, 0.727, 0.727), dur(b,g,a) = 2.271, 1.502, 0.001 ms
I spec common_specu: statistics      ngram-cache: #calls(b,g,a) =    2    957    529, #gen drafts =    529, #acc drafts =   286, #gen tokens =   3390, #acc tokens =   757, #mean acc len = 2.43, #acc rate/pos = (0.541, 0.295, 0.178, 0.127, 0.096, 0.078, 0.062, 0.055), dur(b,g,a) = 0.000, 42.109, 0.072 ms

[petersid2022]

great, thank you !! regarding the point you brought up, I believe that this PR is final as it is but i would be more than happy to begin working on the draft length being used (currently being hard-coded to 8) in a separate PR, by instead making the draft length configurable

[CISC]

Who added merge ready and when? Doesn't show in log...

[Green-Sky]

Who added merge ready and when? Doesn't show in log...

It only shows on mobile.

[ServeurpersoCom]

It was ggerganov at 08:41 UTC today, the API timeline shows but the web log hides it (even on mobile), it's a github webui bug!
I'm reporting it to GitHub through the Maintainer Advisory Network.

[CISC]

🤣 Doesn't even show the merge.

[ServeurpersoCom]

I wonder if this is specific to this PR, or transient bug, it would be bad if there weren't a single source of truth for both the API and the rendering!

[Green-Sky]

Something like this has happened before. A year or two ago.

[CISC]

Something like this has happened before. A year or two ago.

I think more recent as well, definitely not the first time.