Skip to content

vertexai[patch]: ChatVertexAI.generate, stream, bind_tools#166

Merged
Leonid Kuligin (lkuligin) merged 20 commits into
mainfrom
bagatur/refac_bind_tools
Apr 20, 2024
Merged

vertexai[patch]: ChatVertexAI.generate, stream, bind_tools#166
Leonid Kuligin (lkuligin) merged 20 commits into
mainfrom
bagatur/refac_bind_tools

Conversation

@baskaryan
Copy link
Copy Markdown
Contributor

@baskaryan Bagatur (baskaryan) commented Apr 18, 2024

  • Update bind_tools to construct a dict that matches VertexAI Tool proto and bind that to the model
  • Update bind_tools to accept FunctionDeclaration and Vertex Tool directly
  • Update bind_tools to accept a tool_choice param for easier tool_config setting
  • Refactor generate, agenerate, stream, astream to reuse gemini param setting and response parsing

Copy link
Copy Markdown
Contributor

@efriis Erick Friis (efriis) left a comment

Choose a reason for hiding this comment

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

big fan of the gemini/non gemini split

Comment thread libs/vertexai/langchain_google_vertexai/chat_models.py Outdated
@baskaryan Bagatur (baskaryan) changed the title WIP vertexai[patch]: refactor bind_tools vertexai[patch]: ChatVertexAI.generate, stream, bind_tools Apr 18, 2024
@baskaryan Bagatur (baskaryan) marked this pull request as ready for review April 18, 2024 22:50
@baskaryan
Copy link
Copy Markdown
Contributor Author

note: tests/integration_tests/test_chat_models.py::test_chat_vertexai_gemini_function_calling_with_multiple_parts fails non-deterministically

tools: Sequence[Union[Type[BaseModel], Callable, BaseTool]],
tool_config: Optional[Dict[str, Any]] = None,
tools: Sequence[Union[_FunctionDeclarationLike, VertexTool]],
tool_config: Optional[_ToolConfigDict] = None,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nits: should we add to the docstring an example how to contsruct _ToolConfigDict and _ToolChoiceType?

so that people don't need to read the source code.

except (ValueError, IndexError):
return False

def _generate(
Copy link
Copy Markdown
Contributor

@alx13 Alex Ostapenko (alx13) Apr 19, 2024

Choose a reason for hiding this comment

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

+1 for splitting gemini vs non-gemini.

WDYT if we make this method to only select which function to run, and move gemini implementation to _generate_gemini (and obviously in other methods) like this:

    def _generate(
        self,
        messages: List[BaseMessage],
        stop: Optional[List[str]] = None,
        run_manager: Optional[CallbackManagerForLLMRun] = None,
        stream: Optional[bool] = None,
        **kwargs: Any,
    ) -> ChatResult:
        if stream is True or (stream is None and self.streaming):
            stream_iter = self._stream(
                messages, stop=stop, run_manager=run_manager, **kwargs
            )
            return generate_from_stream(stream_iter)
        if not self._is_gemini_model:
            return self._generate_non_gemini(messages, stop=stop, **kwargs)
        return self._generate_gemini(messages, stop=stop, **kwargs)


    def _generate_gemini(
        self,
        messages: List[BaseMessage],
        stop: Optional[List[str]] = None,
        **kwargs: Any,
    ):
        client, contents = self._gemini_client_and_contents(messages)
        params = self._gemini_params(stop=stop, **kwargs)
        with telemetry.tool_context_manager(self._user_agent):
            response = client.generate_content(contents, **params)
        return self._gemini_response_to_chat_result(response)

    def _generate_non_gemini(
        self,
        messages: List[BaseMessage],
        stop: Optional[List[str]] = None,
        **kwargs: Any,
    ) -> ChatResult:
         ...

That would make testing a little bit cleaner.

@lkuligin Leonid Kuligin (lkuligin) deleted the bagatur/refac_bind_tools branch April 20, 2024 16:17
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.

4 participants