[#17020][fix] DeepSeek-V4: honor thinking_budget in apply_chat_template - #17035
Draft
thorjohnsen wants to merge 1 commit into
Draft
[#17020][fix] DeepSeek-V4: honor thinking_budget in apply_chat_template#17035thorjohnsen wants to merge 1 commit into
thorjohnsen wants to merge 1 commit into
Conversation
…template
DeepseekV4Tokenizer.apply_chat_template read only `thinking` /
`enable_thinking`. Every kwarg is fetched with `kwargs.get`, so the
`thinking_budget` key was silently dropped and the request fell back to
non-thinking chat mode.
This matters because `trtllm-eval aime25` and `aime26` default
`--chat_template_kwargs` to `{"thinking_budget": 32768}`, and DeepSeek-V4
ships no Jinja chat template (the checkpoint has no `chat_template` in
tokenizer_config.json), so the hand-written renderer is the only thing that
can honor the key. The result is a run that completes successfully and
reports a plausible-looking score with thinking disabled.
Resolve thinking in one place: `thinking` / `enable_thinking` win whenever
either is passed, so existing behavior is unchanged for callers that use the
native keys; otherwise `thinking_budget` decides, with a positive budget
meaning "think" and 0 disabling it. That matches the semantic the CLI help
text already documents ("set to 0 to disable thinking").
Fixes NVIDIA#17020
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Description
DeepseekV4Tokenizer.apply_chat_templatereads onlythinking/enable_thinking. Every kwarg is fetched withkwargs.get, so an unrecognized key is silently dropped — andthinking_budgetis exactly such a key.That matters because
trtllm-eval aime25andaime26default--chat_template_kwargsto{"thinking_budget": 32768}(lm_eval.py:1513/:1630). DeepSeek-V4 ships no Jinja chat template — the checkpoint has nochat_templateintokenizer_config.json— so the hand-written renderer attokenizer/deepseek_v4/tokenizer.pyis the only thing that could honor the key. Today it does not,thinkingfalls back toFalse, and the prompt ends<｜Assistant｜></think>with the reasoning block pre-closed. The default output is byte-identical to explicitly passing{"thinking": false}.Nothing in the logs or the results indicates this happened. The run completes successfully and reports a plausible-looking score.
Measured on DeepSeek-V4-Flash, AIME 2025, 4xGB300, TP4/EP4, greedy, everything else identical:
--chat_template_kwargsexact_match'{"thinking_budget": 32768}'(current default)'{"thinking": true}'This is genuine capability loss rather than a scoring artifact: 29/30 non-thinking responses did contain a
\boxed{}, and 15 of the 17 failures boxed a confidently wrong value.The change
Thinking resolution moves into one helper.
thinking/enable_thinkingwin whenever the caller passes either, so behavior is unchanged for every caller using the native keys. Only when neither is present doesthinking_budgetdecide: a positive budget means "think",0disables it. That is the semantic the CLI help text already documents ("set to 0 to disable thinking").Reported as #17020. The companion suggestion in that issue — warning on unrecognized
chat_template_kwargskeys — is deliberately left out of this PR to keep it to a single concern; happy to follow up if wanted.Test Coverage
tests/unittest/llmapi/test_deepseek_v4_tokenizer.py, three new cases:test_deepseek_v4_chat_template_supports_thinking_budget— a positive budget opens<think>test_deepseek_v4_chat_template_thinking_budget_zero_disables_thinking—0keeps the block pre-closedtest_deepseek_v4_chat_template_native_thinking_key_wins_over_budget— explicitthinking=Falsebeats a positive budget, pinning the precedence ruleThe existing cases in that file cover the unchanged
thinking/enable_thinking/reasoning_effortpaths.PR Checklist