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

299 ollama client does not work with stream #309

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

BalasubramanyamEvani
Copy link

What does this PR do?

This PR addresses the issue where the application fails to work when the stream parameter is set to True in the adalflow.components.model_client.ollama_client::OllamaClient class. The issue is traced to the adalflow.core.generator.py _post_call method, which does not currently handle streaming correctly.

Fixes #299

  • Implements separate cases for handling streaming and non-streaming responses in the _post_call method.
    • For streaming responses: Processes the raw response from the completion generator as it is yielded and passes it to the output_processors.
    • For non-streaming responses: Retains the existing behavior.

Usage Updates

The updated usage for the stream parameter is as follows:

ollama_ai = {
    "model_client": OllamaClient(host=host),
    "model_kwargs": {
        "model": "phi3:latest",
        "stream": True,
    },
}

generator = Generator(**ollama_ai)
output = generator({"input_str": "What is the capital of France?"})
for chunk in output.data:
    print(chunk.data, end="", flush=True)

# For stream: False
# print(output.data)

Tests Added

  • A new test case, TestGeneratorWithStream, is added in test_generator.py to verify the streaming behavior.
    • The test mocks the GeneratorType response from the parse_chat_completion method and validates the output for streamed data.

Tests output (local) after changes:

======================================================== test session starts =========================================================
platform linux -- Python 3.12.8, pytest-8.3.4, pluggy-1.5.0
rootdir: /home/AdalFlow
configfile: pyproject.toml
plugins: anyio-4.4.0, mock-3.14.0
collected 231 items / 1 skipped                                                                                                      

adalflow/tests/test_AzureClient.py ..                                                                                          [  0%]
adalflow/tests/test_base_data_class.py ..............                                                                          [  6%]
adalflow/tests/test_component.py ...                                                                                           [  8%]
adalflow/tests/test_componentlist.py .............                                                                             [ 13%]
adalflow/tests/test_data.py ...                                                                                                [ 15%]
adalflow/tests/test_data_class_parser.py ......                                                                                [ 17%]
adalflow/tests/test_data_classes.py ....                                                                                       [ 19%]
adalflow/tests/test_data_loader.py .......                                                                                     [ 22%]
adalflow/tests/test_dataclass_object_functions.py ................                                                             [ 29%]
adalflow/tests/test_evaluators.py ..s                                                                                          [ 30%]
adalflow/tests/test_faiss_retriever.py .........                                                                               [ 34%]
adalflow/tests/test_function_expression_parse.py ........................                                                      [ 45%]
adalflow/tests/test_generator.py ......                                                                                        [ 47%]
adalflow/tests/test_generator_call_logger.py ...                                                                               [ 48%]
adalflow/tests/test_grad_component.py ......                                                                                   [ 51%]
adalflow/tests/test_groq_client.py ..                                                                                          [ 52%]
adalflow/tests/test_lancedb_retriver.py ........                                                                               [ 55%]
adalflow/tests/test_lazy_import.py ........                                                                                    [ 59%]
adalflow/tests/test_logger.py ......                                                                                           [ 61%]
adalflow/tests/test_memory.py ...                                                                                              [ 63%]
adalflow/tests/test_model_client.py ...                                                                                        [ 64%]
adalflow/tests/test_ollama_client.py ..                                                                                        [ 65%]
adalflow/tests/test_openai_client.py ..                                                                                        [ 66%]
adalflow/tests/test_output_parser.py ......                                                                                    [ 68%]
adalflow/tests/test_parameter.py ............                                                                                  [ 74%]
adalflow/tests/test_parameter_text_grad.py ...                                                                                 [ 75%]
adalflow/tests/test_random_sample.py .....                                                                                     [ 77%]
adalflow/tests/test_sequential.py ...............                                                                              [ 83%]
adalflow/tests/test_string_parser.py ..........................                                                                [ 95%]
adalflow/tests/test_text_splitter.py .........                                                                                 [ 99%]
adalflow/tests/test_tool.py ..                                                                                                 [100%]

========================================================== warnings summary ==========================================================
adalflow/tests/test_ollama_client.py::TestOllamaModelClient::test_ollama_embedding_client
adalflow/tests/test_ollama_client.py::TestOllamaModelClient::test_ollama_llm_client
  /home/AdalFlow/adalflow/adalflow/components/model_client/ollama_client.py:165: UserWarning: Better to provide host or set OLLAMA_HOST env variable. We will use the default host http://localhost:11434 for now.
    warnings.warn(

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
============================================= 230 passed, 2 skipped, 2 warnings in 3.75s =============================================

Breaking Changes

As far as I can test, this PR does not introduce any breaking changes. Existing functionality for non-streaming cases remains unaffected.


Before submitting
  • Was this discussed/agreed via a GitHub issue? (not for typos and docs)
  • Did you read the contributor guideline?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you make sure to update the documentation with your changes? (if necessary)
  • Did you write any new necessary tests? (not for typos and docs)
  • Did you verify new and existing tests pass locally with your changes?
  • Did you list all the breaking changes introduced by this pull request?

Copy link
Member

@liyin2015 liyin2015 left a comment

Choose a reason for hiding this comment

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

please also check this pr as it is highly related.

Updates in the generator requires to be minimized

#158

@@ -304,24 +313,54 @@ def _extra_repr(self) -> str:
s = f"model_kwargs={self.model_kwargs}, model_type={self.model_type}"
return s

def _process_chunk(self, chunk: Any) -> GeneratorOutput:
Copy link
Member

Choose a reason for hiding this comment

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

it specifies only one output, but you returned a tuple.

Ensure we add code linting @fm1320 by developers to check these basics

Choose a reason for hiding this comment

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

Thanks for. the review. Yes, that's my fault. I will fix this.


return GeneratorOutput(data=process_stream(), raw_response=output)
Copy link
Member

Choose a reason for hiding this comment

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

dont separate the code, it changed too much, and its better to minimize the change, so just add the initial code back to the else

Choose a reason for hiding this comment

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

Understood. I’ll proceed with this approach then. It seems there’s no longer a need for the additional _process_chunk function I had added earlier then?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ollama client (from adalflow.components.model_client.ollama_client) does not work with stream=True
2 participants