forked from Karylab-cklius/vllm
[Bugfix] Fix logprobs token-string collision from SentencePiece space… (#48674)
Signed-off-by: Allen Shen <aoshen@inferact.ai> Co-authored-by: mvanhorn <mvanhorn@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
This commit is contained in:
co-authored by
mvanhorn
Claude Opus 4.6
mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
parent
752bd10647
commit
dcfebf93f4
@@ -8,6 +8,7 @@ import pytest
|
||||
from transformers import AutoTokenizer, PythonBackend, TokenizersBackend
|
||||
|
||||
from vllm.sampling_params import SamplingParams
|
||||
from vllm.tokenizers.detokenizer_utils import convert_ids_list_to_tokens
|
||||
from vllm.tokenizers.mistral import MistralTokenizer
|
||||
from vllm.v1.engine import EngineCoreRequest
|
||||
from vllm.v1.engine.detokenizer import (
|
||||
@@ -239,3 +240,122 @@ def test_oov_decode(tokenizer, fast):
|
||||
|
||||
assert decoded_text == ""
|
||||
assert out_ids == [len(tokenizer)]
|
||||
|
||||
|
||||
# ---------- convert_ids_list_to_tokens collision tests ----------
|
||||
|
||||
|
||||
class _MockBackend:
|
||||
"""Fake backend_tokenizer that exposes pre_tokenizer config."""
|
||||
|
||||
def __init__(self, pre_tokenizer_type, replacement=None):
|
||||
import json
|
||||
|
||||
pre: dict = {"type": pre_tokenizer_type}
|
||||
if replacement is not None:
|
||||
pre["replacement"] = replacement
|
||||
self._config = json.dumps({"pre_tokenizer": pre})
|
||||
|
||||
def to_str(self):
|
||||
return self._config
|
||||
|
||||
|
||||
class _MockTokenizer:
|
||||
"""Minimal tokenizer mock for testing convert_ids_list_to_tokens."""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
raw_tokens: dict[int, str],
|
||||
decoded_tokens: dict[int, str],
|
||||
pre_tokenizer_type: str = "Metaspace",
|
||||
replacement: str | None = "▁",
|
||||
):
|
||||
self._raw = raw_tokens
|
||||
self._decoded = decoded_tokens
|
||||
self.backend_tokenizer = _MockBackend(pre_tokenizer_type, replacement)
|
||||
|
||||
def convert_ids_to_tokens(
|
||||
self, ids: list[int], skip_special_tokens: bool = False
|
||||
) -> list[str]:
|
||||
return [self._raw[tid] for tid in ids]
|
||||
|
||||
def decode(self, ids: list[int], skip_special_tokens: bool = False) -> str:
|
||||
return "".join(self._decoded[tid] for tid in ids)
|
||||
|
||||
|
||||
def test_sentencepiece_leading_space_preserved():
|
||||
"""▁true and true must produce distinct strings."""
|
||||
tok = _MockTokenizer(
|
||||
raw_tokens={0: "▁true", 1: "true", 2: "▁false", 3: "false"},
|
||||
decoded_tokens={0: "true", 1: "true", 2: "false", 3: "false"},
|
||||
)
|
||||
result = convert_ids_list_to_tokens(tok, [0, 1, 2, 3])
|
||||
assert result == [" true", "true", " false", "false"]
|
||||
|
||||
# No dict collision when used as top_logprobs keys
|
||||
logprobs = dict(zip(result, [-0.1, -0.2, -0.3, -0.4]))
|
||||
assert len(logprobs) == 4
|
||||
|
||||
|
||||
def test_whitespace_run_tokens_stay_distinct():
|
||||
"""▁, ▁▁, ▁▁▁ must produce different-length space strings."""
|
||||
tok = _MockTokenizer(
|
||||
raw_tokens={0: "▁", 1: "▁▁", 2: "▁▁▁"},
|
||||
decoded_tokens={0: "", 1: " ", 2: " "},
|
||||
)
|
||||
result = convert_ids_list_to_tokens(tok, [0, 1, 2])
|
||||
assert result == [" ", " ", " "]
|
||||
|
||||
|
||||
def test_bpe_leading_space_already_preserved():
|
||||
"""GPT-2 BPE: Ġtrue already decodes to ' true', no fix needed."""
|
||||
tok = _MockTokenizer(
|
||||
raw_tokens={0: "Ġtrue", 1: "true"},
|
||||
decoded_tokens={0: " true", 1: "true"},
|
||||
pre_tokenizer_type="ByteLevel",
|
||||
replacement=None,
|
||||
)
|
||||
result = convert_ids_list_to_tokens(tok, [0, 1])
|
||||
assert result == [" true", "true"]
|
||||
|
||||
|
||||
def test_logprobs_count_stable_across_k():
|
||||
"""logprobs=4 and logprobs=10 must return 4 and 10 entries."""
|
||||
tok = _MockTokenizer(
|
||||
raw_tokens={
|
||||
0: "▁true",
|
||||
1: "a",
|
||||
2: "b",
|
||||
3: "c",
|
||||
4: "true",
|
||||
5: "d",
|
||||
6: "e",
|
||||
7: "f",
|
||||
8: "g",
|
||||
9: "h",
|
||||
},
|
||||
decoded_tokens={
|
||||
0: "true",
|
||||
1: "a",
|
||||
2: "b",
|
||||
3: "c",
|
||||
4: "true",
|
||||
5: "d",
|
||||
6: "e",
|
||||
7: "f",
|
||||
8: "g",
|
||||
9: "h",
|
||||
},
|
||||
)
|
||||
ids = list(range(10))
|
||||
lps = [-0.1 * (i + 1) for i in range(10)]
|
||||
|
||||
tokens4 = convert_ids_list_to_tokens(tok, ids[:4])
|
||||
top4 = dict(zip(tokens4, lps[:4]))
|
||||
|
||||
tokens10 = convert_ids_list_to_tokens(tok, ids)
|
||||
top10 = dict(zip(tokens10, lps))
|
||||
|
||||
assert len(top4) == 4
|
||||
assert len(top10) == 10
|
||||
assert top4[" true"] == top10[" true"]
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
# SPDX-FileCopyrightText: Copyright contributors to the vLLM project
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
|
||||
from vllm.tokenizers import TokenizerLike
|
||||
|
||||
@@ -56,6 +59,63 @@ def _convert_tokens_to_string_with_added_encoders(
|
||||
INITIAL_INCREMENTAL_DETOKENIZATION_OFFSET = 5
|
||||
|
||||
|
||||
_CACHED_MARKER_KEY = "_vllm_space_marker_cache"
|
||||
_NOT_CACHED = "__not_computed__"
|
||||
|
||||
|
||||
def _get_leading_space_marker(tokenizer: TokenizerLike) -> str | None:
|
||||
"""Read the space marker from the tokenizer's pre_tokenizer config.
|
||||
|
||||
Only Metaspace pre_tokenizers (used by SentencePiece-based models like
|
||||
Llama, Mistral, T5) have a replacement character whose leading instance
|
||||
gets stripped by decode(). ByteLevel (GPT-2), BertPreTokenizer (BERT),
|
||||
and others do not have this issue.
|
||||
|
||||
Returns the marker character, or None if decode() is safe for single
|
||||
tokens.
|
||||
"""
|
||||
cached = getattr(tokenizer, _CACHED_MARKER_KEY, _NOT_CACHED)
|
||||
if cached is not _NOT_CACHED:
|
||||
return cached # type: ignore[return-value]
|
||||
|
||||
backend = getattr(tokenizer, "backend_tokenizer", None)
|
||||
if backend is None:
|
||||
result = None
|
||||
else:
|
||||
result = None
|
||||
try:
|
||||
config = json.loads(backend.to_str())
|
||||
except Exception:
|
||||
pass
|
||||
else:
|
||||
pre = config.get("pre_tokenizer") or {}
|
||||
pre_type = pre.get("type")
|
||||
if pre_type == "Metaspace":
|
||||
result = pre.get("replacement", "▁")
|
||||
elif pre_type == "Sequence":
|
||||
for sub in pre.get("pretokenizers", []):
|
||||
if sub.get("type") == "Metaspace":
|
||||
result = sub.get("replacement", "▁")
|
||||
break
|
||||
|
||||
setattr(tokenizer, _CACHED_MARKER_KEY, result)
|
||||
return result
|
||||
|
||||
|
||||
def _restore_leading_spaces(raw_token: str, token_str: str, marker: str) -> str:
|
||||
"""Restore leading spaces that decode() stripped from a raw vocab piece."""
|
||||
num_markers = 0
|
||||
for ch in raw_token:
|
||||
if ch != marker:
|
||||
break
|
||||
num_markers += 1
|
||||
if num_markers == 0:
|
||||
return token_str
|
||||
existing = len(token_str) - len(token_str.lstrip(" "))
|
||||
missing = num_markers - existing
|
||||
return " " * missing + token_str if missing > 0 else token_str
|
||||
|
||||
|
||||
def convert_prompt_ids_to_tokens(
|
||||
tokenizer: TokenizerLike,
|
||||
prompt_ids: list[int],
|
||||
@@ -86,6 +146,10 @@ def convert_ids_list_to_tokens(
|
||||
) -> list[str]:
|
||||
"""Detokenize the input ids individually.
|
||||
|
||||
Uses decode() for human-readable output, then checks the raw vocab
|
||||
piece via convert_ids_to_tokens() to restore any leading spaces that
|
||||
decode() stripped (SentencePiece add_dummy_prefix inverse).
|
||||
|
||||
Args:
|
||||
tokenizer: tokenizer used by model under test
|
||||
token_ids: convert these tokens (Python list form)
|
||||
@@ -94,14 +158,16 @@ def convert_ids_list_to_tokens(
|
||||
Python list of token string representations
|
||||
|
||||
"""
|
||||
token_str_lst = []
|
||||
for token_id in token_ids:
|
||||
# use default skip_special_tokens.
|
||||
token_str = tokenizer.decode([token_id])
|
||||
if token_str is None:
|
||||
token_str = ""
|
||||
token_str_lst.append(token_str)
|
||||
return token_str_lst
|
||||
if not token_ids:
|
||||
return []
|
||||
marker = _get_leading_space_marker(tokenizer)
|
||||
if marker is None:
|
||||
return [tokenizer.decode([tid]) or "" for tid in token_ids]
|
||||
raw_tokens = tokenizer.convert_ids_to_tokens(token_ids)
|
||||
return [
|
||||
_restore_leading_spaces(raw, tokenizer.decode([tid]) or "", marker)
|
||||
for tid, raw in zip(token_ids, raw_tokens)
|
||||
]
|
||||
|
||||
|
||||
# Based on
|
||||
|
||||
Reference in New Issue
Block a user