feat(gemma4): cross the sliding window — front-release with retained global - #887
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f60224f5d5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| pub(crate) fn advance_and_release(&mut self, count: usize, window: usize) -> Result<()> { | ||
| let page = self.pool.layout().page_size; | ||
| let frontier = self.frontier + count; | ||
| let target = frontier.saturating_sub(window) / page; |
There was a problem hiding this comment.
At a post-step frontier of 1039 with window = 1024 and page = 16, the next query is position 1039 and its mask starts at position 16, so page 0 (tokens 0–15) is already unreachable. frontier.saturating_sub(window) / page nevertheless retains it until frontier 1040; this occurs for one step at every page boundary and can consume an unnecessary page per request or refuse otherwise admissible work under pool pressure. Derive the target from frontier - (window - 1) or an equivalent inclusive-window calculation.
Useful? React with 👍 / 👎.
f60224f to
90c5e99
Compare
|
@codex review please |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90c5e99291
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if max_abs > tolerance { | ||
| over.push(format!("{label} ({max_abs} > {tolerance})")); | ||
| } |
There was a problem hiding this comment.
When a window-crossing regression changes token rankings but keeps the reference tokens' log-probability deltas below the deliberately broad backend-derived tolerance, this gate still passes: score_rows computes top1, but the value is only printed and never asserted. In particular, the 4096-token case permits several log-probability units of error, so even zero top-1 matches could remain green; compare top1 against a calibrated floor such as the sdpa/eager self-agreement already available in the fixture.
Useful? React with 👍 / 👎.
Signed-off-by: Feathbow <feathbow@gmail.com>
90c5e99 to
80a6148
Compare
|
@codex review please |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
1 participant
Description
Closes #886
The local family becomes a sliding state: resident pages held as single-page reservations, released from the front once
(page + 1) * 16 + window <= frontier— after the step's attention, since append-then-attend keeps the old window and the new tokens co-resident through the layers. What stops growing is the logical page count a request holds once a step completes; the step itself still peaks at everything it wrote, and the pool's device buffer is allocated up front either way, so a release returns pages to the pool rather than to the driver. The paged preps take the resident row plus apage_origin: the origin is page-aligned, so in-page offsets stay position-invariant and only the row index shifts. RoPE keeps absolute positions; the attention plan lives in cache-relative coordinates, wherewindow_leftmasks whatever sub-window prefix the first page still carries — a non-aligned resident start loses nothing.Test Env
Single GPU (sm_89, x86_64), CUDA 12.9, pinned 12B checkpoint,
--test-threads=1Verification
-D warningsacross gemma4/core/kernels targets — clean.ceil(kv_len / 16) - floor((kv_len - window) / 16)resident pages, so releasing one page late fails the gate.(origin + 1) * 16 + window <= frontier, i.e. at frontier 1040, so w1023/w1024/w1025 (1031/1032/1033 tokens after their forced steps) gate the mask boundary —window_leftstarting to hide the oldest keys — while w4096 is the case that releases, and it is where the footprint claim comes from (local 65 pages against global 257). The eviction A/B runs at the same shorter length, so its margin is one page (65 vs 66); the wide margin lives in w4096.Type of Change