Feature/refactor generate - #1722
Open
michalk8 wants to merge 25 commits into
Open
Conversation
Apertus emits tool calls as an array of single key objects wrapped in
<|tools_prefix|> ... <|tools_suffix|>, e.g.
<|tools_prefix|>[{"get_weather": {"location": "London"}}]<|tools_suffix|>
Adds a parser for that format and infers it from the chat template, so
Apertus models work without an explicit tool_parser_type.
Also fixes reasoning detection. Apertus delimits deliberation with
<|inner_prefix|> ... <|inner_suffix|>, but carries unused <think>/</think>
tokens in its vocabulary, so _infer_thinking matched those instead and the
reasoning state was never reachable — deliberation was returned as content
rather than reasoning.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
zcbenz
reviewed
Aug 18, 2026
zcbenz
left a comment
zcbenz
left a comment
Member
There was a problem hiding this comment.
Is it possible to split the PR into smaller ones?
| # Shift left to reduce padding | ||
| min_left_pad = self.left_padding.min().item() | ||
| mx.eval(self.left_padding) | ||
| min_left_pad = min(self.left_padding.tolist()) |
Member
There was a problem hiding this comment.
What this change does is moving computation of min from gpu to cpu?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR refactors some choices in the
generate.pyfile and fixes (in some part) the issues below:insert_segments validates option lengths up front and commits only after every sequence is built; stream_generate rejects max_tokens=0 rather than crashing. Raising is the intended contract, not the empty stream the issue suggests.
Ours names the offending option and is atomic. Its max_tokens < 0 guard would close off the working unbounded range (verified at -1 and -7).
Same crash fixed, opposite contract: max_tokens=0 raises, because it already means prefill-only in generate_step and an empty stream would silently skip the prefill.
On more surface too — BatchGenerator.insert, not just batch_generate.
_normalize_logits_processors turns None into () at insert, so it can't reach the per-token loop.
Guarded tps properties in generate_utils.py; stats()'s finally only accumulates, so it can't mask the body's exception.
keep > 0 guard moved into _merge_caches; the unconditional keep=4 blocking --kv-bits stands.
Also fixes some issues in the KV cache's left padding (syncing too often); now does
self.left_padding.tolist()[idx]instead of computing this usingmx).