Skip to content

Custom Model Client support#1345

Merged
sonichi merged 55 commits into
microsoft:mainfrom
olgavrou:custom_client
Feb 2, 2024
Merged

Custom Model Client support#1345
sonichi merged 55 commits into
microsoft:mainfrom
olgavrou:custom_client

Conversation

@olgavrou

@olgavrou olgavrou commented Jan 19, 2024

Copy link
Copy Markdown
Contributor

Why are these changes needed?

This PR adds support for custom client calls to be made. The idea is that the user can specify their own custom client class and load it into the configuration (e.g. for loading local models). As long as they adhere to the Client class's interface and response protocol then everything should work fine with only this addition to the config (full usage shown in the unit test):

So if we have the class CustomClient the class name (as a string) needs to be set as the api_type in the config:

config_list = [
    {
        "model": "local_model_name",
        "model_client_cls" = "CustomClient",
        "device": "cuda",
        "params": {
            "max_length": 1000,
            "temperature": 1.1,
            [other params that will be consumed in the Custom Client so user has full control over what is set and what is consumed]
        }
    }
]

Then once an agent is created the custom client can be registered:

agent.register_custom_client(CustomClient, [optional other args for the constuctor of CustomClient])

Notes:
Ideally the client interface would not live under a directory called oai and the client wrapper wouldn't be called OpenAIWrapper but something more generic, but that can be examined at a later date

Related PR

Replaces #831 and has less refactoring done so that the change is more contained

Checks

@codecov-commenter

codecov-commenter commented Jan 19, 2024

Copy link
Copy Markdown

Codecov Report

Attention: 97 lines in your changes are missing coverage. Please review.

Comparison is base (b0c1f8d) 33.12% compared to head (1ea55cc) 46.59%.

❗ Current head 1ea55cc differs from pull request most recent head 479696c. Consider uploading reports for the commit 479696c to get more accurate results

Files Patch % Lines
autogen/oai/client.py 39.62% 88 Missing and 8 partials ⚠️
autogen/agentchat/conversable_agent.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1345       +/-   ##
===========================================
+ Coverage   33.12%   46.59%   +13.47%     
===========================================
  Files          42       42               
  Lines        5051     5112       +61     
  Branches     1157     1239       +82     
===========================================
+ Hits         1673     2382      +709     
+ Misses       3250     2530      -720     
- Partials      128      200       +72     
Flag Coverage Δ
unittests 46.53% <40.49%> (+13.45%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sonichi sonichi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment thread autogen/agentchat/conversable_agent.py Outdated
Comment thread autogen/oai/client.py Outdated
Comment thread autogen/oai/client.py
@sonichi

sonichi commented Feb 1, 2024

Copy link
Copy Markdown
Contributor

All the tests are passed except GPTAssistantAgent: https://github.com/microsoft/autogen/actions/runs/7733107408/job/21084512475?pr=1345

@davorrunje davorrunje left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall, this is great! I have a few stylistic suggestions to make it a bit more pythonic.

Comment thread autogen/oai/client.py
Comment thread autogen/oai/client.py Outdated
Comment thread autogen/oai/client.py Outdated
Comment thread autogen/oai/client.py
Comment thread autogen/oai/client.py
Comment thread autogen/oai/client.py
Comment thread autogen/oai/client.py
Comment thread autogen/oai/client.py Outdated
@davorrunje

davorrunje commented Feb 1, 2024

Copy link
Copy Markdown
Contributor

@olgavrou I fixed the failing tests by introducing an additional protocol AssistantModelClient extending ModelClient with two new properties required for GPT assistants to work.

You can see my changes here:
7c68924

If you wish to merge it with your branch:

git checkout custom_client
git fetch
git merge origin/fix-tests-custom-client

There are a few todos left in the code to make type checks and autocomplete tighter, but functionally this is it.

@olgavrou

olgavrou commented Feb 2, 2024

Copy link
Copy Markdown
Contributor Author

All the tests are passed except GPTAssistantAgent: https://github.com/microsoft/autogen/actions/runs/7733107408/job/21084512475?pr=1345

@sonichi

added a fix for it, it needs access to the internal client that was now refactored to belong to the OpenAIClient class. I will open a PR after this is merged to refactor that test a bit so that it doesn't access internal private member vars

@ekzhu

ekzhu commented Feb 3, 2024

Copy link
Copy Markdown
Contributor

Just want to say I am so happy it is merged finally!

@marklysze

Copy link
Copy Markdown
Contributor

Fantastic work, thank you

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.

8 participants