Skip to content
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

feat: Add streaming to HuggingFaceLocalGenerator #7377

Merged
merged 3 commits into from
Mar 21, 2024
Merged

Conversation

vblagoje
Copy link
Member

Why:

The change introduces a new streaming_callback parameter to the HuggingFaceLocalGenerator class, allowing users to handle streaming responses. This enhancement provides greater flexibility and customization in managing real-time responses from Hugging Face's local models.

What:

  1. Modifies haystack/components/generators/hugging_face_local.py:

    • Adds streaming_callback as an optional parameter to the HuggingFaceLocalGenerator class.
    • Implements streaming_callback functionality within the run method.
    • Updates to_dict and from_dict methods to serialize and deserialize the streaming_callback parameter.
  2. Adds releasenotes/notes/hugging-face-local-generator-streaming-callback-38a77d37199f9672.yaml to document the new feature.

  3. Modifies test/components/generators/test_hugging_face_local_generator.py to include tests for the streaming_callback parameter.

How can it be used:

  1. Include streaming_callback when initializing a HuggingFaceLocalGenerator instance to handle streaming responses:
    generator = HuggingFaceLocalGenerator(model="model_name", streaming_callback=streaming_callback_handler)
  2. Implement the streaming_callback function to handle streaming responses as desired, for example:
    def streaming_callback_handler(x):
        print(x)

How did you test it:

  1. Tested the streaming_callback by creating tests for:
    • Default behavior (no streaming callback)
    • Serialization and deserialization (to_dict and from_dict methods)
    • Custom streaming callback function implementation
    • Interaction with the run method

Notes for the reviewer:

  • Ensure that the streaming_callback function is compatible with the async context of the HuggingFaceLocalGenerator's run method.
  • Test the streaming feature with local Hugging Face models to validate the behavior.
  • Test the serialization and deserialization functionality of the streaming_callback parameter.
  • Consider adding integration tests for various Hugging Face models to ensure compatibility with different types of streaming data.

@vblagoje vblagoje requested review from a team as code owners March 18, 2024 11:19
@vblagoje vblagoje requested review from dfokina and davidsbatista and removed request for a team March 18, 2024 11:19
@github-actions github-actions bot added topic:tests 2.x Related to Haystack v2.0 type:documentation Improvements on the docs labels Mar 18, 2024
@vblagoje vblagoje changed the title feat: Add streaming_callback parameter to the HuggingFaceLocalGenerator feat: Add streaming to HuggingFaceLocalGenerator Mar 18, 2024
@coveralls
Copy link
Collaborator

coveralls commented Mar 18, 2024

Pull Request Test Coverage Report for Build 8375520678

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 6 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.02%) to 89.28%

Files with Coverage Reduction New Missed Lines %
components/generators/hugging_face_local.py 6 91.89%
Totals Coverage Status
Change from base Build 8374477800: 0.02%
Covered Lines: 5422
Relevant Lines: 6073

💛 - Coveralls



# used to test serialization of streaming_callback
def streaming_callback_handler(x):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it makes more sense to define this inside the test_run_with_streaming(self) itself, since it's only used there

Copy link
Member Author

Choose a reason for hiding this comment

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

Good suggestion @davidsbatista - I would but the serialization mechanism doesn't see the namespace of the function IIRC.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nevermind, it can be done, thanks for the suggestion

Copy link
Contributor

@davidsbatista davidsbatista left a comment

Choose a reason for hiding this comment

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

just a small suggestions, other than that LGTM

@vblagoje vblagoje force-pushed the hf_generator_streaming branch from 7e9460c to 12e63ad Compare March 21, 2024 13:10
@vblagoje
Copy link
Member Author

@davidsbatista rebased to latest main, resolved a few conflicts, should be ready to go now, just waiting for green signal from CI

@vblagoje vblagoje merged commit e779d43 into main Mar 21, 2024
23 checks passed
@vblagoje vblagoje deleted the hf_generator_streaming branch March 21, 2024 14:49
silvanocerza pushed a commit that referenced this pull request Apr 8, 2024
* Inital streaming impl

* Add unit tests

* Add release note
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x Related to Haystack v2.0 topic:tests type:documentation Improvements on the docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HuggingFaceLocalGenerator - streaming/stream handler support
3 participants