Skip to content

fix: preserve per-member caches with memory saving - #1365

Merged
jmkuebler merged 2 commits into
mainfrom
jonas/fix_cached_attention_memory_saving
Oct 12, 2026
Merged

jmkuebler merged 2 commits into
mainfrom
jonas/fix_cached_attention_memory_saving

Conversation

@jmkuebler

@jmkuebler jmkuebler commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Jonas: We had a bug when using memory saving mode together with cached predictions since the batching PR #1312. We are now not using memory saving mode anymore for cached predictions. Those memory peaks can anyways be bound by setting the maximum number of test query rows when prediction with cache. so another saving is not needed imo.


Generated by Codex

Memory-saving chunking split cached v3/v3.5 queries without slicing their K/V caches, so later ensemble members used the first member's cache and repeatedly dequantized the full batch. Keep cached attention on the intact member batch while retaining MLP memory saving and inference-engine test-row chunking. Add a tiny CPU regression test for both architectures with plain and int8 caches.

Issue

Fixes #1364.

Public API Changes

  • No public API changes.

How Has This Been Tested?

All four new regression cases fail before the fix and pass afterward. The 18 small cache unit tests pass; Ruff and pre-commit hooks pass. No checkpoint download or GPU is needed for the new test.

Reproduction and runtime evidence

The regression test uses two members, five cached rows, one query, and eight features. Distinct cache values make the wrong-member lookup observable. The attention output projection is nonzero so initialization cannot hide the defect.

The prior targeted RTX PRO 6000 experiment tested the equivalent cached-attention change on 200,000 training rows × 58 columns, eight estimators, and a single cached query:

Setting Median predict Class-1 probability
Original batching with memory saving 115.511 ms 0.7800719142
Corrected cached attention 49.141 ms 0.9951955080
Unbatched reference 77.517 ms 0.9951962829

The corrected and original batched settings reused the same fitted cache. The correction replaces 64 single-member ICL attention calls with eight correctly aligned batch calls. Other memory-saving operations remain enabled. No full accuracy benchmark was run.

Checklist

  • Tested locally.
  • Public API unchanged; no usage documentation change needed.
  • Changelog entry added.
  • Style checks pass.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f780182. Configure here.

Comment thread changelog/1365.fixed.md
@jmkuebler
jmkuebler requested review from bejaeger and removed request for bejaeger October 12, 2026 06:25

@bejaeger bejaeger left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jmkuebler
jmkuebler added this pull request to the merge queue Oct 12, 2026
Merged via the queue into main with commit 8eea89e Oct 12, 2026
20 checks passed
@jmkuebler
jmkuebler deleted the jonas/fix_cached_attention_memory_saving branch October 12, 2026 12:10
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.

Cached ensemble predictions use the wrong cache with memory saving enabled

2 participants