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

Portkey Integration with Autogen #3395

Merged
merged 23 commits into from
Sep 9, 2024
Merged

Conversation

siddharthsambharia-portkey
Copy link
Contributor

@qingyun-wu @marklysze

Why are these changes needed?

Created documentation for Integrating Portkey with Autogen. It provides a brief overview of Portkey's features and explains how it can be used to bring AutoGen agents into production.

Related issue number

Closes #3394

Checks

@siddharthsambharia-portkey
Copy link
Contributor Author

@microsoft-github-policy-service agree company="Portkey"

Copy link
Collaborator

@thinkall thinkall left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @siddharthsambharia-portkey . Could you also provide a notebook example so it

website/docs/ecosystem/portkey.md Outdated Show resolved Hide resolved
@siddharthsambharia-portkey
Copy link
Contributor Author

Thanks for the PR, @siddharthsambharia-portkey . Could you also provide a notebook example so it
The PR already has a notebook showcasing a hands-on example of integrating Portkey with Autogen, check out our notebook

Google Colab .

@gagb gagb requested a review from jackgerrits August 23, 2024 18:54
@gagb
Copy link
Collaborator

gagb commented Aug 23, 2024

@jackgerrits -- can we please double check the language for ecosystem.

@marklysze
Copy link
Collaborator

marklysze commented Aug 23, 2024

@siddharthsambharia-portkey, thanks so much for the contribution!

I have a few questions if I can regarding non-OpenAI providers (such as Anthropic, Cohere, Groq, Mistral, etc.):

  • How do you do provider-specification configuration parameters (e.g. top_k) - is it in the targets list/dictionary of the portkey config?
  • Have you tested multiple providers with AutoGen? The reason I ask is, that in my experiences from creating specific client classes, some of these clients are particular on things like the role within messages, not wanting name to be on messages.
  • Does tool/function calling work with different providers? (Update - I can see the features supported in the Portkeys docs, noted) Will it load balance/fallback to a provider that doesn't support tool calling when the request contains a tool?

I tried the sample code but I couldn't get it to balance/fallback. I wanted it to use Anthropic and Groq, both in the same group chat of the sample code. Is there anyway to test that? At the moment I'm trying to understand which AutoGen client class it is going to use when it load balances / fallbacks between different providers.

Co-authored-by: gagb <gagb@users.noreply.github.com>
Co-authored-by: gagb <gagb@users.noreply.github.com>
Co-authored-by: gagb <gagb@users.noreply.github.com>
@siddharthsambharia-portkey
Copy link
Contributor Author

@siddharthsambharia-portkey, thanks so much for the contribution!

I have a few questions if I can regarding non-OpenAI providers (such as Anthropic, Cohere, Groq, Mistral, etc.):

  • How do you do provider-specification configuration parameters (e.g. top_k) - is it in the targets list/dictionary of the portkey config?
  • Have you tested multiple providers with AutoGen? The reason I ask is, that in my experiences from creating specific client classes, some of these clients are particular on things like the role within messages, not wanting name to be on messages.
  • Does tool/function calling work with different providers? (Update - I can see the features supported in the Portkeys docs, noted) Will it load balance/fallback to a provider that doesn't support tool calling when the request contains a tool?

I tried the sample code but I couldn't get it to balance/fallback. I wanted it to use Anthropic and Groq, both in the same group chat of the sample code. Is there anyway to test that? At the moment I'm trying to understand which AutoGen client class it is going to use when it load balances / fallbacks between different providers.

For provider-specific configuration parameters like top_k, you can add these fields as override_params in the config object. These are specified in the targets list/dictionary of the Portkey config. link to config Docs- https://docs.portkey.ai/docs/api-reference/config-object
Here's an example of how it would look:

JSON

{
  "strategy": {
    "mode": "fallback"
  },
  "targets": [
    {
      "provider": "groq",
      "api_key": "groq-api-key",
      "override_params": {
        "top_k": "0.4",
        "max_tokens": "100"
      }
    },
    {
      "provider": "anthropic",
      "api_key": "anthropic-api-key",
      "override_params": {
        "top_k": "0.6",
        "model": "claude-3-5-sonnet-20240620"
      }
    }
  ]
}
  1. Regarding multiple providers with AutoGen, yes, it works well with non-OpenAI providers that support function calling.

  2. Tool/function calling will work with providers that support it.
    Regarding load balancing/fallback to providers that don't support tool calling when the request contains a function call, the user needs to manage this carefully. Ideally you should group similar LLMs (in terms of function calling support) for fallback/load balancing to ensure compatibility.

@siddharthsambharia-portkey
Copy link
Contributor Author

@gagb @marklysze I have made the resolved all the comments, is there anything else required to merge the PR?

Thanks!

@gagb gagb self-requested a review August 26, 2024 07:09
Copy link
Collaborator

@gagb gagb left a comment

Choose a reason for hiding this comment

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

Consider extending the write up with your answers to the questions that @marklysze asked-- others are likely to have similar questions. Otherwise looks good to go.

@siddharthsambharia-portkey
Copy link
Contributor Author

Consider extending the write up with your answers to the questions that @marklysze asked-- others are likely to have similar questions. Otherwise looks good to go.

Yes, I will update the write-up and update Portkey docs for the same as well!
Thanks!

@marklysze
Copy link
Collaborator

Do you know if there's a way to handle this?

Hey @marklysze this is Anthropic specific problem, they throw an error if there is an assistant message after the system message. Portkey is an AI Gateway, It can't change anything in the request. There is no way to handle this using Portkey

Yes, appreciate the challenge here (as we've had to create specific client/provider classes to cater for these cases).

@siddharthsambharia-portkey and @gagb, do we need to make a note of the preference for OpenAI API compatible providers to ensure better compatibility?

@gagb
Copy link
Collaborator

gagb commented Aug 27, 2024

Do you know if there's a way to handle this?

Hey @marklysze this is Anthropic specific problem, they throw an error if there is an assistant message after the system message. Portkey is an AI Gateway, It can't change anything in the request. There is no way to handle this using Portkey

Yes, appreciate the challenge here (as we've had to create specific client/provider classes to cater for these cases).

@siddharthsambharia-portkey and @gagb, do we need to make a note of the preference for OpenAI API compatible providers to ensure better compatibility?

Yes that sounds like a good idea (at least for the current version of AutoGen).

@siddharthsambharia-portkey
Copy link
Contributor Author

Do you know if there's a way to handle this?

Hey @marklysze this is Anthropic specific problem, they throw an error if there is an assistant message after the system message. Portkey is an AI Gateway, It can't change anything in the request. There is no way to handle this using Portkey

Yes, appreciate the challenge here (as we've had to create specific client/provider classes to cater for these cases).

@siddharthsambharia-portkey and @gagb, do we need to make a note of the preference for OpenAI API compatible providers to ensure better compatibility?

Yes, we could do that ideally but the models keep changing every few months, you can never be certain. I think the user can decide what model works best for his use case
.

@siddharthsambharia-portkey
Copy link
Contributor Author

Do you know if there's a way to handle this?

Hey @marklysze this is Anthropic specific problem, they throw an error if there is an assistant message after the system message. Portkey is an AI Gateway, It can't change anything in the request. There is no way to handle this using Portkey

Yes, appreciate the challenge here (as we've had to create specific client/provider classes to cater for these cases).
@siddharthsambharia-portkey and @gagb, do we need to make a note of the preference for OpenAI API compatible providers to ensure better compatibility?

Yes, we could do that ideally but the models keep changing every few months, you can never be certain. I think the user can decide what model works best for his use case .

@marklysze @gagb Is there anything else you are waiting for to merge this PR?

@marklysze
Copy link
Collaborator

marklysze commented Aug 28, 2024

Do you know if there's a way to handle this?

Hey @marklysze this is Anthropic specific problem, they throw an error if there is an assistant message after the system message. Portkey is an AI Gateway, It can't change anything in the request. There is no way to handle this using Portkey

Yes, appreciate the challenge here (as we've had to create specific client/provider classes to cater for these cases).
@siddharthsambharia-portkey and @gagb, do we need to make a note of the preference for OpenAI API compatible providers to ensure better compatibility?

Yes, we could do that ideally but the models keep changing every few months, you can never be certain. I think the user can decide what model works best for his use case .

@marklysze @gagb Is there anything else you are waiting for to merge this PR?

My only concern without making any mention of a possible incompatibility due to the messaging structure is users raising issues when they get an incompatible role order. I think the wording can be simple, perhaps something like.
"Note: AutoGen messages will be passed to Portkey in line with OpenAI's API and this may be not be compatible with some LLMs that require messages to be in a certain role order."

@siddharthsambharia-portkey
Copy link
Contributor Author

ible incompatibility due to the messaging structure is users raising issues when they get an incompatible rol

Do you know if there's a way to handle this?

Hey @marklysze this is Anthropic specific problem, they throw an error if there is an assistant message after the system message. Portkey is an AI Gateway, It can't change anything in the request. There is no way to handle this using Portkey

Yes, appreciate the challenge here (as we've had to create specific client/provider classes to cater for these cases).
@siddharthsambharia-portkey and @gagb, do we need to make a note of the preference for OpenAI API compatible providers to ensure better compatibility?

Yes, we could do that ideally but the models keep changing every few months, you can never be certain. I think the user can decide what model works best for his use case .

@marklysze @gagb Is there anything else you are waiting for to merge this PR?

My only concern without making any mention of a possible incompatibility due to the messaging structure is users raising issues when they get an incompatible role order. I think the wording can be simple, perhaps something like. "Note: AutoGen messages will be passed to Portkey in line with OpenAI's API and this may be not be compatible with some LLMs that require messages to be in a certain role order."

You're right; this could impact the user experience. However, Portkey serves as an AI Gateway, allowing users to easily switch between different models without altering the request process. Alternatively Autogen can recommend the best models for agents, and Portkey can facilitate using those models. This setup gives users flexibility to change models as new ones become available or for A/B testing, like switching from OpenAI to Llama 3.1

@gagb
Copy link
Collaborator

gagb commented Aug 29, 2024

ible incompatibility due to the messaging structure is users raising issues when they get an incompatible rol

Do you know if there's a way to handle this?

Hey @marklysze this is Anthropic specific problem, they throw an error if there is an assistant message after the system message. Portkey is an AI Gateway, It can't change anything in the request. There is no way to handle this using Portkey

Yes, appreciate the challenge here (as we've had to create specific client/provider classes to cater for these cases).
@siddharthsambharia-portkey and @gagb, do we need to make a note of the preference for OpenAI API compatible providers to ensure better compatibility?

Yes, we could do that ideally but the models keep changing every few months, you can never be certain. I think the user can decide what model works best for his use case .

@marklysze @gagb Is there anything else you are waiting for to merge this PR?

My only concern without making any mention of a possible incompatibility due to the messaging structure is users raising issues when they get an incompatible role order. I think the wording can be simple, perhaps something like. "Note: AutoGen messages will be passed to Portkey in line with OpenAI's API and this may be not be compatible with some LLMs that require messages to be in a certain role order."

You're right; this could impact the user experience. However, Portkey serves as an AI Gateway, allowing users to easily switch between different models without altering the request process. Alternatively Autogen can recommend the best models for agents, and Portkey can facilitate using those models. This setup gives users flexibility to change models as new ones become available or for A/B testing, like switching from OpenAI to Llama 3.1

@siddharthsambharia-portkey -- can you please fix the code formatting errors using pre-commit run all-files and also added @marklysze 's suggestion and lets get this merged :)

@siddharthsambharia-portkey
Copy link
Contributor Author

ible incompatibility due to the messaging structure is users raising issues when they get an incompatible rol

Do you know if there's a way to handle this?

Hey @marklysze this is Anthropic specific problem, they throw an error if there is an assistant message after the system message. Portkey is an AI Gateway, It can't change anything in the request. There is no way to handle this using Portkey

Yes, appreciate the challenge here (as we've had to create specific client/provider classes to cater for these cases).
@siddharthsambharia-portkey and @gagb, do we need to make a note of the preference for OpenAI API compatible providers to ensure better compatibility?

Yes, we could do that ideally but the models keep changing every few months, you can never be certain. I think the user can decide what model works best for his use case .

@marklysze @gagb Is there anything else you are waiting for to merge this PR?

My only concern without making any mention of a possible incompatibility due to the messaging structure is users raising issues when they get an incompatible role order. I think the wording can be simple, perhaps something like. "Note: AutoGen messages will be passed to Portkey in line with OpenAI's API and this may be not be compatible with some LLMs that require messages to be in a certain role order."

You're right; this could impact the user experience. However, Portkey serves as an AI Gateway, allowing users to easily switch between different models without altering the request process. Alternatively Autogen can recommend the best models for agents, and Portkey can facilitate using those models. This setup gives users flexibility to change models as new ones become available or for A/B testing, like switching from OpenAI to Llama 3.1

@siddharthsambharia-portkey -- can you please fix the code formatting errors using pre-commit run all-files and also added @marklysze 's suggestion and lets get this merged :)

sure that's awesome. I will do that right away

@siddharthsambharia-portkey
Copy link
Contributor Author

cc @gagb @marklysze

@gagb
Copy link
Collaborator

gagb commented Aug 29, 2024

cc @gagb @marklysze

@siddharthsambharia-portkey, marks suggest text is still not there? Can we please add that :)

@gagb gagb requested a review from marklysze August 29, 2024 23:27
@siddharthsambharia-portkey
Copy link
Contributor Author

cc @gagb @marklysze

@siddharthsambharia-portkey, marks suggest text is still not there? Can we please add that :)

Yes my bad, added it. @gagb

Copy link
Collaborator

@marklysze marklysze left a comment

Choose a reason for hiding this comment

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

Looks good @siddharthsambharia-portkey, thanks!

@marklysze
Copy link
Collaborator

@siddharthsambharia-portkey, can you please:

  • remove the trailing white-space on line 15 after 2.
  • add a blank line at the end of the file

@siddharthsambharia-portkey
Copy link
Contributor Author

done @marklysze, there was alredy a blank line at the end of the file although

@siddharthsambharia-portkey
Copy link
Contributor Author

Hey @gagb @marklysze LGTM?

@gagb gagb enabled auto-merge September 9, 2024 05:33
@gagb gagb added this pull request to the merge queue Sep 9, 2024
Merged via the queue into microsoft:main with commit 5f87b28 Sep 9, 2024
19 checks passed
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.

[Feature Request]: Portkey Integration with Autogen
4 participants