Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions keras_hub/src/tokenizers/sentence_piece_tokenizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,13 +149,25 @@ def _set_proto_tf(self, proto):

def _set_proto_spm(self, proto):
self._sentence_piece_spm = spm.SentencePieceProcessor()
self._sentence_piece_spm.Init(
model_proto=proto,
out_type=str if is_string_dtype(self.compute_dtype) else int,
add_bos=self.add_bos,
add_eos=self.add_eos,
alpha=1.0,
)
out_type = str if is_string_dtype(self.compute_dtype) else int

if hasattr(self._sentence_piece_spm, "Init"):
# Older SWIG wrapper (<0.2.2)
self._sentence_piece_spm.Init(
model_proto=proto,
out_type=out_type,
add_bos=self.add_bos,
add_eos=self.add_eos,
alpha=1.0,
)
else:
# Newer PyBind11 wrapper (>=0.2.2)
self._sentence_piece_spm = spm.SentencePieceProcessor(
model_proto=proto,
out_type=out_type,
add_bos=self.add_bos,
add_eos=self.add_eos,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

In sentencepiece >= 0.2.2 (which uses the PyBind11 wrapper), the SentencePieceProcessor constructor does not accept out_type, add_bos, or add_eos as arguments. Passing these arguments will raise a TypeError (e.g., TypeError: __init__() got an unexpected keyword argument 'out_type').

Additionally, since the Init method is removed in the PyBind11 wrapper, we cannot set a default out_type on the processor instance. If we do not pass out_type to Encode, it will default to integer IDs even when self.compute_dtype is a string type, leading to incorrect tokenization behavior.

To fix this robustly without modifying code outside the diff hunks, we can use a lightweight delegation wrapper for the PyBind11 processor that intercepts calls to Encode and injects the correct out_type.

Suggested change
else:
# Newer PyBind11 wrapper (>=0.2.2)
self._sentence_piece_spm = spm.SentencePieceProcessor(
model_proto=proto,
out_type=out_type,
add_bos=self.add_bos,
add_eos=self.add_eos,
)
else:
# Newer PyBind11 wrapper (>=0.2.2)
class PyBind11Processor:
def __init__(self, processor, out_type):
self._processor = processor
self._out_type = out_type
def Encode(self, *args, **kwargs):
if "out_type" not in kwargs:
kwargs["out_type"] = self._out_type
return self._processor.Encode(*args, **kwargs)
def __getattr__(self, name):
return getattr(self._processor, name)
processor = spm.SentencePieceProcessor()
processor.LoadFromSerializedProto(proto)
self._sentence_piece_spm = PyBind11Processor(processor, out_type)
References
  1. Poke Holes in the Implementation & Demand Robustness: Actively search for and point out failing edge cases or unhandled exceptions in the implementation, and do not accept fragile code. (link)


def set_proto(self, proto):
if proto is None:
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ dependencies = [
"rich",
"kagglehub!=1.0.1",
"kagglesdk==0.1.28",
"sentencepiece<0.2.2",
"sentencepiece",
"tokenizers",
"tensorflow-text;platform_system != 'Windows'",
]
Expand Down
2 changes: 1 addition & 1 deletion requirements-common.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ rich
kagglehub!=1.0.1
kagglesdk==0.1.28
tokenizers
sentencepiece<0.2.2
sentencepiece
# Tooling deps.
pre-commit
astor
Expand Down
Loading