fix: DRY sampler never applies during decoding (stale token history views)#1720
fix: DRY sampler never applies during decoding (stale token history views)#1720Nakanokensetsu wants to merge 1 commit into
Conversation
AlpinDale
left a comment
AlpinDale
left a comment
There was a problem hiding this comment.
Hi, and thanks for the PR. However it seems that unrelated changes were included in this PR?
| @@ -218,7 +218,22 @@ def _process_scanner_items(self, items: Sequence[LexerInput]) -> list[SemanticEv | |||
| events: list[SemanticEvent] = [] | |||
| for item in items: | |||
| if isinstance(item, PreLexedTerminal): | |||
| events.extend(self._process_lex_tokens(self._lexer.flush())) | |||
| # The text lexer may be holding a buffered prefix (e.g. "<tool_") | |||
There was a problem hiding this comment.
Is this change related to this PR?
There was a problem hiding this comment.
Same as above, seems unrelated.
| @@ -709,9 +699,15 @@ def _cache_partial_tail_block(self, request: Request, num_tokens: int) -> None: | |||
| block_idx = boundary_tokens // self.block_size | |||
| if block_idx >= len(blocks): | |||
| return | |||
| block = blocks[block_idx] | |||
| token_history_ids_cpu, token_history_lens_cpu = ( | ||
| self._get_token_history_cpu_views(num_reqs) if not self.no_dry else (None, None) | ||
| ) | ||
| # DRY CPU-kernel fast path disabled (2026-07-25): the cached |
There was a problem hiding this comment.
git blame already offers us with commit timestamps so no need to include it here.
| @@ -1415,6 +1421,10 @@ def _make_output_token_ids_tensor(self, num_reqs: int) -> torch.Tensor: | |||
| return output_token_ids_cpu_tensor.to(device=self.device, non_blocking=True) | |||
|
|
|||
| def _get_token_history_cpu_views(self, num_reqs: int) -> tuple[torch.Tensor, torch.Tensor]: | |||
| # NOTE (2026-07-25): unused — the DRY CPU-kernel path is disabled in | |||
There was a problem hiding this comment.
Might as well delete this function instead of leaving dead code lying around.
| @@ -438,3 +440,56 @@ def test_partial_block_promotes_to_direct_full_block_hash(): | |||
| ) | |||
| assert pool.get_cached_block(promoted_full_hash, [kv_cache_group_id]) == [blocks[1]] | |||
| assert pool.get_cached_block(partial_hash_10, [kv_cache_group_id]) is None | |||
|
|
|||
|
|
|||
| def test_manager_does_not_resurrect_partial_hash_after_full_promotion(): | |||
There was a problem hiding this comment.
I feel like this should be another PR, same as above.
db1238f to
6ab8200
Compare
…iews) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
6ab8200 to
6b533d5
Compare
|
You were right — the branch was cut from my fork's Also dropped the date from the comment and deleted If you would rather fix the CPU kernel path than bypass it, that means rebuilding the views at sampling time instead of metadata-build time — happy to do that instead, it was just a much larger change. |
2 participants
Problem
The DRY sampler is effectively inert during real decoding while appearing fully configured. With any repetition-inducing prompt,
dry_multiplier=5.0anddry_multiplier=0.0(same seed) produce bit-identical outputs.Root cause
InputBatch._make_sampling_metadata()materializes the CPU-kernel inputs aswith
max_history_lencomputed at metadata build time. Sampling metadata is only rebuilt on batch-composition changes, so during steady-state decoding the view's width stays frozen at (roughly) the prompt length. Every generated token is written outside the view;dry_scan_penaltiesclamps each row's length to the tensor width and therefore rescans the prompt-only prefix forever (instrumentation shows per-row lens growing 36→49 while the history tensor shape stays(1, 36), and the kernel returns the identical penalty set every step).Net effect: DRY yields either no penalty at all, or a frozen prompt-derived penalty, for the entire generation.
Why the obvious fix doesn't work
Widening the cached view to full width makes it live (row-only slices share storage), but at sampling time the tail of
token_ids_cpucontains async-scheduling placeholder ids (-1). The kernel then matches the placeholder run and emits penalties for token index-1(observed:token_indexes=[-1],match_lens=[65]), i.e. it penalizes garbage.Fix
Stop feeding the CPU kernel and let
Sampler.apply_dryfall through to the persistent-state python path, which is updated with the real sampled ids each step viaupdate_dry_stateand computes correct penalties.Verification (TP=2, Qwen3.5-family 9B, optimization-level 3)
dry=0.0→ 25 verbatim repetitions,dry=5.0→ 25 verbatim repetitions (identical bytes)dry=0.0→ 25 repetitions,dry=5.0→ 0 repetitionsdry=0/5(previously bit-identical end to end).Happy to adjust if you'd rather repair the CPU-kernel path instead (it would need per-step view refresh plus masking of the async placeholder tail).
🤖 Generated with Claude Code