Skip to content

Python : #6499 Initial Commit for Mistral Connector #6921

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.

Sign up for GitHub

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

Jump to bottom
Closed
nmoeller wants to merge 21 commits into from
Closed

Python : #6499 Initial Commit for Mistral Connector #6921

nmoeller wants to merge 21 commits into from

Conversation

@nmoeller
Copy link
Contributor

@nmoeller nmoeller commented Jun 24, 2024 •
edited
Loading

Motivation and Context

  1. Why is this changed required ?
    To enable Mistral Models with Semantic Kernel, there was the issue Python: Add Mistral AI Connector #6499 in the Backlog to add a MistralAI Connector
  2. What problem does it solve ?
    It solves the problem, that semantic kernel is not yet integrated with MistralAI
  3. What scenario does it contribute to?
    The scenario is to use different connector than HF,OpenAI or AzureOpenAI. When User's want to use Mistral they can easliy integrate it now
  4. If it fixes an open issue, please link to the issue here.
    Python: Add Mistral AI Connector #6499

Description

The changes made are designed by the open_ai connector, i tried to stay as close as possible to the structure.
For the integration i installed the mistral python package in the repository.

I added the following Classes :

  • MistrealAIChatPromptExcecutionSettings --> Responsible to administrate the Prompt Execution against MistralAI
  • MistralAIChatCompletion --> Responsible to coordinate the Classes and for Content parsing
  • MistralAISettings --> Basic Settings to work with the MistralAIClient

To test the changes with the tests please add MISTRALAI_API_KEY as Enviorment Variable

From a design decision i moved the processing of Functions from Connectors to the ChatCompletionClientBaseClass

What is integrated yet :

  • Integrate Mistral AI Chat Models without Streaming
  • Integrate Mistral AI Chat Models with Streaming
  • Simple Integration Test to test Streaming and non Streaming
  • Integrate MistralAI Models with Embeddings
  • Integrate MistralAI FunctionCalling
  • Extended Testing including Unit Testing & More Integration Tests

Contribution Checklist

@markwallace-microsoft markwallace-microsoft added the python Pull requests for the Python Semantic Kernel label Jun 24, 2024
@markwallace-microsoft
Copy link
Member

markwallace-microsoft commented Jun 24, 2024 •
edited
Loading

Py3.10 Test Coverage

Python 3.10 Test Coverage Report •
FileStmtsMissCoverMissing
semantic_kernel/connectors/ai/open_ai/services
   open_ai_chat_completion_base.py1687058%100, 104, 124, 149–153, 178, 186, 188, 192, 210–215, 233–261, 264–275, 290–297, 308–316, 332–339, 360, 368, 374–377, 389–392, 423
semantic_kernel/functions
   kernel_function_from_prompt.py154795%165–166, 180, 200, 218, 238, 321
TOTAL668678188% 

Python 3.10 Unit Test Overview

Tests Skipped Failures Errors Time
1567 1 💤 0 ❌ 0 🔥 18.502s ⏱️

@nmoeller
Copy link
Contributor Author

nmoeller commented Jun 24, 2024

@microsoft-github-policy-service agree

moonbox3
moonbox3 reviewed Jun 24, 2024
View reviewed changes
Copy link
Collaborator

@moonbox3 moonbox3 left a comment

Choose a reason for hiding this comment

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

This is looking nice. Thank you for taking on the work and for your contribution.

@moonbox3
Copy link
Collaborator

moonbox3 commented Jun 24, 2024

Can you please comment on the following work item so that I can assign it to you? Thank you.

eavanvalkenburg
eavanvalkenburg requested changes Jun 27, 2024
View reviewed changes
Copy link
Member

@eavanvalkenburg eavanvalkenburg left a comment

Choose a reason for hiding this comment

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

some initial comments, seems like their design is very similar to OpenAI, I do not believe that that means we should reuse code, we don't want to get into a situation where OpenAI or Mistral changes something and we have to re-separate code!

@nmoeller nmoeller marked this pull request as ready for review July 1, 2024 07:48
@nmoeller nmoeller requested a review from a team as a code owner July 1, 2024 07:48
@nmoeller nmoeller requested a review from moonbox3 July 2, 2024 11:48
@nmoeller
Copy link
Contributor Author

nmoeller commented Jul 2, 2024

With seeing this #7035 here, it looks like @TaoChenOSU is also going for an implementation of the function calling inside the connector. If we both try to implement shared Function Calling Code, this will probably result into merge conflicts.

I think i will close this and will submit a new PR which is very lightweight and easy to understand for the Mistral Connector.
And then we can define a new Issue if we want to merge the FunctionCalling Flow of all connectors.

@nmoeller nmoeller closed this Jul 2, 2024
@nmoeller nmoeller mentioned this pull request Jul 2, 2024
Merged
8 tasks
@nmoeller nmoeller deleted the issue-6499-Mistral-Ai-Connector branch August 14, 2024 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eavanvalkenburg eavanvalkenburg eavanvalkenburg requested changes

@moonbox3 moonbox3 Awaiting requested review from moonbox3

Assignees

No one assigned

Labels

python Pull requests for the Python Semantic Kernel

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@nmoeller @markwallace-microsoft @moonbox3 @eavanvalkenburg @nmoellerms