Skip to content

UPSTREAM PR #18730: lookahead: fix n_seq_max and kv_unified configuration - #874

Open
loci-dev wants to merge 1 commit into
mainfrom
upstream-PR18730-branch_pestopoppa-fix-lookahead-n-seq-max
Open

UPSTREAM PR #18730: lookahead: fix n_seq_max and kv_unified configuration#874
loci-dev wants to merge 1 commit into
mainfrom
upstream-PR18730-branch_pestopoppa-fix-lookahead-n-seq-max

Conversation

@loci-dev

Copy link
Copy Markdown

Mirrored from ggml-org/llama.cpp#18730

Summary

Fixes llama-lookahead configuration issues that have been broken since PR #14482 (July 2025).

Note: This PR depends on #18729 for the batch init fix. Both PRs are needed for lookahead to fully work.

Root Cause

Two lookahead-specific configuration issues:

1. Sequence count (n_seq_max)

PR #14482 changed seq_id validation from LLAMA_MAX_SEQ (large constant) to n_seq_max (context-specific). Lookahead needs W + G + 1 = 31 sequences for parallel Jacobi decoding, but params.n_parallel defaulted to 1.

2. KV unified mode

Batch splitting with "coupled sequences" requires unified KV cache. Lookahead didn't enable this, causing:

split_equal: sequential split is not supported when there are coupled sequences

Fix

// lookahead requires W + G + 1 sequences for parallel Jacobi decoding
params.n_parallel = W + G + 1;

// unified KV cache is required for coupled sequences in batch splitting
params.kv_unified = true;

Bug Timeline

Date PR Effect
Nov 2023 #4207 lookahead.cpp created - works
July 2025 #14482 seq_id validation changed - breaks lookahead

Testing

With both this PR and #18729 applied:

encoded    4 tokens in    0.138 seconds
decoded   51 tokens in   71.591 seconds
n_accept  = 13

Dependencies

  • Requires #18729 for the batch init fix (params.n_ctxllama_n_ctx(ctx))

Bug history researched with Claude.

llama-lookahead has been broken since PR #14482 (July 2025) which changed
seq_id validation from LLAMA_MAX_SEQ constant to context-specific n_seq_max.

Two lookahead-specific issues:

1. n_seq_max: Lookahead needs W + G + 1 = 31 sequences for parallel Jacobi
   decoding, but params.n_parallel defaulted to 1.
   Fix: Set params.n_parallel = W + G + 1 before context creation.

2. KV unified: Batch splitting with coupled sequences requires unified KV
   cache mode, but lookahead didn't enable it.
   Fix: Set params.kv_unified = true.

Bug timeline:
- Nov 2023: lookahead.cpp created, worked with LLAMA_MAX_SEQ constant
- July 2025: PR #14482 changed to n_seq_max validation, broke lookahead

Note: This PR depends on #18729 for the batch init fix (params.n_ctx ->
llama_n_ctx). Both PRs are needed for lookahead to fully work.

Tested with Qwen2.5-Coder-0.5B: lookahead generates output with n_accept > 0.

Bug history researched with Claude.
@loci-review

loci-review Bot commented Jan 10, 2026

Copy link
Copy Markdown

Explore the complete analysis inside the Version Insights

Perfect! I was able to retrieve the summary report for your project. Here's what the analysis shows:

Summary Report for llama.cpp PR #874

Project Information:

Key Findings:

No significant performance changes detected

The performance analysis comparing the two versions shows that:

  • No modified functions exhibited performance changes greater than 2% in response time
  • No modified functions exhibited performance changes greater than 2% in throughput time

Conclusion:
The changes introduced in pull request #874 maintain performance stability with minimal to no measurable impact on the llama.cpp binary's performance characteristics. This is a positive result indicating that the code modifications don't introduce performance regressions.

Would you like me to provide more detailed information about specific functions or any other aspect of this comparison?

@loci-dev
loci-dev force-pushed the main branch 27 times, most recently from 8e509d5 to 63d526f Compare January 13, 2026 23:09
@loci-dev
loci-dev force-pushed the main branch 30 times, most recently from 48924ee to fb5dc2f Compare January 21, 2026 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants