open-standards/jouleclaw/jouleclaw-rs/crates/jouleclaw-omni/tests/ggml_kv_cache.rs
Claude 950634f200
All checks were successful
standards workspaces build + test / which workspaces this change touches (pull_request) Successful in 7s
jouleclaw fabrics / cuda (linux, RTX 4050 + Vulkan loader) (pull_request) Successful in 16s
jouleclaw fabrics / vulkan (linux, RTX 4050 + Vulkan loader) (pull_request) Successful in 20s
standards workspaces build + test / mesh-rs (mesh/mesh-rs, -p mesh-engine-gguf -p mesh-openai --features mesh-engine-gguf/gguf,mesh-openai/server, mesh-rs, linux-arm64) (pull_request) Successful in 1m39s
repository checks / lockfiles, tests in CI, workflow hygiene, spec and section references, public text (pull_request) Successful in 1m58s
standards workspaces build + test / jouleclaw-rs (-nodebug, jouleclaw/jouleclaw-rs, -p jouleclaw-energy --features rapl,nvml, jouleclaw-rs, linux-arm64, -C target-feature=+fp16) (pull_request) Successful in 4m46s
jouleclaw public surface must stay documented / Public-surface audit (jouleclaw-rs) (pull_request) Successful in 1m2s
jouleclaw fabrics / metal (macOS, Apple GPU) (pull_request) Successful in 1m12s
standards workspaces build + test / macOS energy backends (pull_request) Successful in 9m13s
standards workspaces build + test / jouledesk-rs (jouledesk/jouledesk-rs, jouledesk-rs, macos) (pull_request) Successful in 2m54s
ggml decode: the KV cache was sized by two literals the write site ignored
`DecodeBuffers::new` built the KV cache with

    let max_seq = 4096usize;
    let num_attn_layers = 6; // Nemotron: 6 attention layers

while `encode_decode_step` indexes it with the model's real attention-layer
ordinal and the real position:

    let attn_idx = layers.iter().take(layer.idx + 1)
        .filter(|l| l.layer_type == LayerType::Attention).count() - 1;
    let dst_offset = _position as u64 * kv_row_f16;

Two consequences, neither of which reported anything:

- A model with a seventh attention layer got `attn_idx == 6`, the
  `if attn_idx < bufs.kv_k_cache.len()` guard was false, and that layer's
  K and V were simply never written. Attention then read a zero-filled
  cache: wrong output, no error.
- `_position` is never bounded anywhere in the function, so a position at
  or past 4096 gave `set_buffer` an offset past the end of the buffer.

Both lengths now come from the model. `init` resolves the layer types and
passes the attention-layer count; `max_seq` comes from
`config.max_seq_len`, falling back only when it is 0, which would
otherwise allocate a zero-length cache that fails every write. The
capacity is recorded on `DecodeBuffers` so the decode path can check
rather than assume, and `encode_decode_step` now checks both once, up
front, where an error can propagate -- the per-site `if idx < len` guards
are deep inside nested closures where a `return Err` cannot.

tests/ggml_kv_cache.rs pins the two lengths: one buffer per attention
layer for counts of 1, 6, 7 and 11 (6 being the literal that hid this),
buffer length following max_seq_len for 512, 4096 and 8192, and a
zero-length config still producing a usable cache.

Scope: this path is behind the GGML_GRAPH environment variable, so it is
opt-in rather than the default decode path.

Still assumed, and worth separate attention: `GRAPH` and `LAYERS_CACHE`
are process-global `OnceLock`s, so a second model loaded in the same
process reuses the first one's buffers and layer pattern.

Co-Authored-By: Claude Opus 5 <[email protected]>
2026-10-20 16:32:06 -04:00

86 lines
3.4 KiB
Rust

//! The ggml decode path's KV cache is indexed by attention-layer ordinal and
//! by position. Both were literals -- 6 layers and 4096 positions -- while the
//! write site used the model's real ordinal and the real position, so a model
//! with more attention layers had its writes dropped by an `if idx < len`
//! guard, and a position past 4096 addressed past the end of the buffer.
//!
//! These tests pin the two lengths against what was asked for. They need a
//! Metal device and do not skip without one.
#![cfg(feature = "metal")]
use jouleclaw_omni::hal::metal::MetalDevice;
use jouleclaw_omni::inference::ggml_graph::DecodeBuffers;
use jouleclaw_omni::inference::model::ModelConfig;
/// A small but complete hybrid config. Every field below is read by
/// `DecodeBuffers::new` to size a buffer, and Metal rejects a zero-length
/// allocation, so none of them may be left at its default of 0.
fn config(num_layers: usize, max_seq_len: usize) -> ModelConfig {
ModelConfig {
num_layers,
max_seq_len,
hidden_size: 64,
num_heads: 4,
num_kv_heads: 2,
intermediate_size: 128,
num_experts: 4,
num_active_experts: 2,
ssm_inner_size: 32,
ssm_state_size: 8,
ssm_group_count: 2,
ssm_time_step_rank: 4,
ssm_conv_kernel: 4,
vocab_size: Some(1024),
..Default::default()
}
}
#[test]
fn kv_cache_has_one_buffer_per_attention_layer() {
let device = MetalDevice::new().expect("a Metal device is required by these tests");
// The count is the caller's, not a constant: 6 was the literal that made a
// 7th attention layer's writes vanish.
for attn_layers in [1usize, 6, 7, 11] {
let bufs = DecodeBuffers::new(device.raw(), &config(52, 4096), attn_layers);
assert_eq!(
bufs.kv_k_cache.len(), attn_layers,
"K cache must hold one buffer per attention layer"
);
assert_eq!(
bufs.kv_v_cache.len(), attn_layers,
"V cache must hold one buffer per attention layer"
);
assert_eq!(bufs.kv_attn_layers, attn_layers, "recorded capacity must match");
}
}
#[test]
fn kv_cache_length_follows_the_config_not_a_literal() {
let device = MetalDevice::new().expect("a Metal device is required by these tests");
// 4096 was the literal. A model configured for more must get more, or its
// later positions write past the end of the buffer.
for max_seq in [512usize, 4096, 8192] {
let bufs = DecodeBuffers::new(device.raw(), &config(52, max_seq), 2);
assert_eq!(bufs.kv_max_seq, max_seq, "recorded length must follow the config");
// Two bytes per F16 element, num_kv_heads * head_dim elements per row.
let head_dim = 64 / 4;
let row = 2 * head_dim * 2;
assert_eq!(
bufs.kv_k_cache[0].length() as usize, max_seq * row,
"each buffer must be long enough for every position"
);
}
}
#[test]
fn a_config_with_no_max_seq_len_still_allocates() {
let device = MetalDevice::new().expect("a Metal device is required by these tests");
// max_seq_len == 0 would allocate a zero-length buffer and fail every
// write, so it falls back rather than producing an unusable cache.
let bufs = DecodeBuffers::new(device.raw(), &config(52, 0), 2);
assert!(bufs.kv_max_seq > 0, "a zero-length KV cache is never usable");
assert!(bufs.kv_k_cache[0].length() > 0);
}