-
Notifications
You must be signed in to change notification settings - Fork 208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make TextStreamer
public & add unit-tests
#1700
base: master
Are you sure you want to change the base?
Make TextStreamer
public & add unit-tests
#1700
Conversation
if (text.size() >= 3 && text.compare(text.size() - 3, 3, replacement) == 0) { | ||
m_decoded_lengths[m_decoded_lengths.size() - 1] = -1; | ||
// Don't print incomplete text | ||
return set_streaming_status(on_finalized_subword_callback(res.str())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IterableStreamer
needs a similar change. Please add ChunkStreamer
to tests as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
Turned out
IterableStreamer
already handles this situation correctly. -
We talked with @as-suvorov, looks like he will add such unit-tests together with implementation of
TextStreamer
with vectorized write.
220815c
to
c30f1a8
Compare
1667b9e
to
8d9c781
Compare
TextStreamer
public & add unit-tests
@@ -131,6 +143,11 @@ PYBIND11_MODULE(py_openvino_genai, m) { | |||
.def(py::init<>()) | |||
.def("write", &StreamerBase::write, "Write is called every time new token is decoded. Returns a StreamingStatus flag to indicate whether generation should be stopped or cancelled", py::arg("token")) | |||
.def("end", &StreamerBase::end, "End is called at the end of generation. It can be used to flush cache if your own streamer has one"); | |||
|
|||
py::class_<TextStreamer, std::shared_ptr<TextStreamer>>(m, "TextStreamer", text_streamer_docstring) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to highlight that TextStreamerBase
is a base class
I would also create a dedicated py_text_streamer.cpp
file for all streamers on python side
# ("black-forest-labs/FLUX.1-dev", dict(subfolder="tokenizer")), # FLUX.1-dev has tokenizer in subfolder | ||
] | ||
|
||
# Chekc that fix for CVS-157216 works. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Chekc that fix for CVS-157216 works. | |
# Check that fix for CVS-157216 works. |
callback: User-defined callback function to process the decoded text, callback should return either boolean flag or StreamingStatus. | ||
|
||
""" | ||
def __init__(self, tokenizer: ..., callback: typing.Callable[[str], bool | StreamingStatus]) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type ...
is unexpected
Tokenizer
class should be defined before TextStreamer
in pybind11 PYBIND11_MODULE(py_openvino_genai, ..)
... | ||
def end(self) -> None: | ||
... | ||
def write(self, arg0: int) -> StreamingStatus: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please, use explicit argument name via py::arg
to avoid automatic arg0
names
__all__ = ['Adapter', 'AdapterConfig', 'AggregationMode', 'AutoencoderKL', 'CLIPTextModel', 'CLIPTextModelWithProjection', 'CacheEvictionConfig', 'ChunkStreamerBase', 'ContinuousBatchingPipeline', 'CppStdGenerator', 'DecodedResults', 'EncodedResults', 'FluxTransformer2DModel', 'GenerationConfig', 'GenerationResult', 'Generator', 'Image2ImagePipeline', 'ImageGenerationConfig', 'InpaintingPipeline', 'LLMPipeline', 'PerfMetrics', 'RawPerfMetrics', 'SD3Transformer2DModel', 'Scheduler', 'SchedulerConfig', 'StopCriteria', 'StreamerBase', 'StreamingStatus', 'T5EncoderModel', 'Text2ImagePipeline', 'TokenizedInputs', 'Tokenizer', 'TorchGenerator', 'UNet2DConditionModel', 'VLMPipeline', 'WhisperGenerationConfig', 'WhisperPerfMetrics', 'WhisperPipeline', 'WhisperRawPerfMetrics', 'draft_model', 'get_version', 'openvino', 'os', 'py_openvino_genai'] | ||
__version__: str | ||
__all__ = ['Adapter', 'AdapterConfig', 'AggregationMode', 'AutoencoderKL', 'CLIPTextModel', 'CLIPTextModelWithProjection', 'CacheEvictionConfig', 'ChunkStreamerBase', 'ContinuousBatchingPipeline', 'CppStdGenerator', 'DecodedResults', 'EncodedResults', 'FluxTransformer2DModel', 'GenerationConfig', 'GenerationResult', 'Generator', 'Image2ImagePipeline', 'ImageGenerationConfig', 'InpaintingPipeline', 'LLMPipeline', 'PerfMetrics', 'RawPerfMetrics', 'SD3Transformer2DModel', 'Scheduler', 'SchedulerConfig', 'StopCriteria', 'StreamerBase', 'StreamingStatus', 'T5EncoderModel', 'Text2ImagePipeline', 'TextStreamer', 'TokenizedInputs', 'Tokenizer', 'TorchGenerator', 'UNet2DConditionModel', 'VLMPipeline', 'WhisperGenerationConfig', 'WhisperPerfMetrics', 'WhisperPipeline', 'WhisperRawPerfMetrics', 'draft_model', 'get_version', 'openvino', 'os', 'py_openvino_genai'] | ||
__version__: str = '2025.1.0.0-1760-6e22001f986-add_text_streamer_tests' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
having a version is unexpected
it's explicitly removed to avoid regeneration of this file for each commit because of different hashes
m_decoded_lengths
relevant information that text in the position of this token is incomplete (is expressed with-1
).TextStreamer
public.TextStreamer
which use pybind to make available this object from Python for test purposes. ButTextCallbackStreamer
is still private and visible only for developers.Ticket: CVS-148635