Skip to content

UPSTREAM PR #18729: lookup, lookahead: fix crash when n_ctx not specified - #875

Open
loci-dev wants to merge 1 commit into
mainfrom
upstream-PR18729-branch_pestopoppa-fix-lookup-lookahead-batch-init
Open

UPSTREAM PR #18729: lookup, lookahead: fix crash when n_ctx not specified#875
loci-dev wants to merge 1 commit into
mainfrom
upstream-PR18729-branch_pestopoppa-fix-lookup-lookahead-batch-init

Conversation

@loci-dev

Copy link
Copy Markdown

Mirrored from ggml-org/llama.cpp#18729

Summary

Fixes a crash in llama-lookup and llama-lookahead when run without explicit -c flag:

GGML_ASSERT(batch.seq_id[batch.n_tokens] && "llama_batch size exceeded")

Root Cause

Both examples use params.n_ctx directly for batch initialization:

// lookup.cpp:109
llama_batch batch_tgt = llama_batch_init(params.n_ctx, 0, 1);

// lookahead.cpp:118
llama_batch batch = llama_batch_init(params.n_ctx, 0, W + G + 1);

Since #16653 changed the default n_ctx to 0 (for GPU auto-fitting), params.n_ctx remains 0 even after the context is properly initialized. This creates a zero-sized batch that crashes on the first common_batch_add().

Bug History

This bug was dormant for 2+ years:

Date PR Default n_ctx Effect
Nov 2023 #4207 512 lookahead.cpp created - works
Dec 2023 #4484 512 lookup.cpp created - works
Nov 2024 #10136 4096 Default increased - works
Dec 2025 #16653 0 Auto-fitting enabled - CRASHES

The pattern was always incorrect, but only triggered when n_ctx default became 0.

Fix

Use llama_n_ctx(ctx) to get the actual runtime context size:

// Before
llama_batch batch_tgt = llama_batch_init(params.n_ctx, 0, 1);

// After
llama_batch batch_tgt = llama_batch_init(llama_n_ctx(ctx), 0, 1);

This matches:

  • The pattern already used in lookup.cpp:72 for max_context_size
  • The pattern used in speculative.cpp and speculative-simple.cpp

Testing

# Before fix (crashes):
llama-lookup -m model.gguf -f prompt.txt --draft 4 -n 50
# GGML_ASSERT(batch.seq_id[batch.n_tokens] && "llama_batch size exceeded")

# After fix (works):
llama-lookup -m model.gguf -f prompt.txt --draft 4 -n 50
# n_accept = 1, accept = 12.500%

Note

llama-lookahead has a separate pre-existing issue with sequence initialization (n_seq_max=1 when it needs W+G+1) that is unrelated to this batch size fix.

Since PR #16653 (Dec 15, 2025), the default n_ctx is 0 to enable automatic
GPU memory fitting. This causes llama-lookup and llama-lookahead to crash
when run without explicit -c flag:

    GGML_ASSERT(batch.seq_id[batch.n_tokens] && "llama_batch size exceeded")

Root cause: Both examples use params.n_ctx directly for batch initialization,
but params.n_ctx remains 0 even after the context is properly initialized
to n_ctx_train internally.

Bug history:
- Nov 2023: lookahead.cpp created (PR #4207) with params.n_ctx pattern
- Dec 2023: lookup.cpp created (PR #4484) with same pattern
- Nov 2024: default n_ctx changed to 4096 (PR #10136) - bug dormant
- Dec 2025: default n_ctx changed to 0 (PR #16653) - bug activated

The bug was dormant for 2+ years because params.n_ctx defaulted to 512,
then 4096. PR #16653 changed it to 0 for GPU auto-fitting, triggering
the crash.

Fix: Use llama_n_ctx(ctx) to get the actual runtime context size, matching
the pattern already used elsewhere in lookup.cpp (line 72) and in
speculative.cpp/speculative-simple.cpp.

Tested: llama-lookup now works without -c flag (12.5% acceptance on
Gemma-3-1B).

Note: llama-lookahead has a separate pre-existing issue with sequence
initialization (n_seq_max=1 vs W+G+1 needed) that is unrelated to this fix.
@loci-review

loci-review Bot commented Jan 10, 2026

Copy link
Copy Markdown

Explore the complete analysis inside the Version Insights

I've generated a summary report for your project. The analysis shows that Pull Request #875 for the llama.cpp repository (owned by auroralabs-loci) has no significant performance regressions.

Key highlights:

  • ✅ No modified functions show performance changes greater than 2%
  • ✅ Both response time and throughput time remain stable
  • ✅ The PR appears safe to merge from a performance perspective

The comparison between the base version (b3ffacf1-edb0-11f0-a055-c529586b3e1a) and target version (b675fbc1-edbe-11f0-a055-c529586b3e1a) indicates this is a performance-neutral change with no concerning degradation.

@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