fix(sglang): size batch truncation from the longest input in the batch, not the first - #1355
Open
shoemoney wants to merge 1 commit into
Open
Conversation
GenerativeTaskDataset sorts each split by character length of the query (src/lighteval/data.py:250), so inputs[0] is the longest prompt in characters but not necessarily in tokens. _greedy_until sized its truncation check from len(inputs[0]) alone, so a batch whose first prompt is short in tokens skips truncation entirely and any longer-in-tokens prompt later in the batch is sent to the sglang engine above max_length. Same defect shape as the vllm copy: confirmed in issue huggingface#1204 and fixed by PR huggingface#1205, which touches only src/lighteval/models/vllm/vllm_model.py and leaves this identical sglang twin unfixed. This mirrors that fix: context_size = max((len(input_ids) for input_ids in inputs), default=0). Adds a unit test with a batch where the character-longest prompt is not the token-longest; it fails on main (a 60-token input goes through untruncated against max_length=50) and passes with the fix.
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
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.
SGLangModel._greedy_untildecides whether to truncate a whole batch fromcontext_size = len(inputs[0])(src/lighteval/models/sglang/sglang_model.py:271 on main). ButGenerativeTaskDatasetsorts each split by character length of the query (src/lighteval/data.py:250,-(len(query) + gen_length)), soinputs[0]is the longest prompt in characters, not necessarily in tokens. A batch whose first prompt is short in tokens (long ASCII text) skips truncation entirely, and a later prompt that is shorter in characters but longer in tokens (for example CJK text) is sent to the sglang engine abovemax_length, corrupting that sample's result.This is the same defect that issue #1204 confirmed for the vllm backend, whose fix PR #1205 makes its model-side change in
src/lighteval/models/vllm/vllm_model.py. Lines 271-292 of the sglang model mirror vllm_model.py:372-397 line for line except for the max_length None guard discussed below, so this PR applies the identical one-line fix to the sglang twin:The added unit test builds a two-doc batch where the character-longest prompt tokenizes to 10 tokens and the character-shorter one to 60, with
max_length=50andgeneration_size=10. On main it fails with70 not less than or equal to 50(the 60-token input reaches_generateuntruncated); with the fix every input is left-truncated to 40 tokens and it passes.ruff format --checkandruff checkare clean.Deliberately not changed here: unlike the vllm copy, the sglang path has no
self.max_length is Noneguard before the comparison._create_auto_modeldefaults_max_lengthto 8192 when unset, so the None case appears unreachable for sglang, and adding the guard would widen this PR beyond the confirmed defect.