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

[Bug]: speaker_selection_agent does not get registered in GroupChat if the selector is a custom model client #2643

Closed
stevenshinechen opened this issue May 10, 2024 · 9 comments · Fixed by #2696
Labels
0.2 Issues which are related to the pre 0.4 codebase group chat/teams group-chat-related issues help wanted Extra attention is needed

Comments

@stevenshinechen
Copy link

stevenshinechen commented May 10, 2024

Describe the bug

When creating a group chat with speaker_selection_method="auto" and the group chat manager registered as a custom model client, the speaker_selection_agent gets the custom llm_config, but is not registered with the custom model client.
Thus we get a model client not activated error when trying to use the speaker_selection_agent at

response = llm_client.create(

Here the speaker_selection_agent is not registered:

speaker_selection_agent = ConversableAgent(

This also occurs in other places where an agent is created for you.
For example, in Teachability, self.analyzer is created for you without registration of custom model.

Steps to reproduce

  1. Create a GroupChat with speaker_selection_method="auto"
  2. Create a GroupChatManager with a custom model client as llm_config
  3. Register the GroupChatManager with the custom model client
  4. Initiate a chat with the GroupChatManager
  5. See model client not activated error

Model Used

Custom model

Expected Behavior

speaker_selection_agent should be registered with the same model client as the selector

Screenshots and logs

image
Custom LLM client does not get registered here

Additional Information

No response

@ekzhu ekzhu added the group chat/teams group-chat-related issues label May 10, 2024
@ekzhu
Copy link
Collaborator

ekzhu commented May 10, 2024

Thanks for pointing this out. What do you think about exposing the select_speaker_agent either through constructor or an attribute, so we can better customize the llm_config and its client.

cc @WaelKarkoub @marklysze

@stevenshinechen
Copy link
Author

Although exposing select_speaker_agent would solve this problem in this case, it is a general issue with agents being initialised for you in methods (internal helper agents that are not created by the developer but in the library).

For example, the self.analyzer created for Teachability (

self.analyzer = TextAnalyzerAgent(llm_config=self.llm_config)
)

Is not registered to a model client for custom models.

I'm sure there are other places where an agent is used internally but is not registered.

One solution could be to store a list of registered custom model client classes, then when an internal agent is used, it would register all the model client classes in that list.

If we were to expose the internal agents, this would have to be done for all internal agents - which might be revealing too much about internal implementation detail if the implementation were to be changed in the future. Although it would add more customizability as it means the internal agents could have a different client to the outer agent so it is a trade-off.

@ekzhu
Copy link
Collaborator

ekzhu commented May 13, 2024

One solution could be to store a list of registered custom model client classes, then when an internal agent is used, it would register all the model client classes in that list.

This is a good idea. We can start by simply passing the client of the parent agent to the internal agent, starting from the group chat.

@ekzhu ekzhu added the help wanted Extra attention is needed label May 13, 2024
@Matteo-Frattaroli
Copy link

Matteo-Frattaroli commented May 15, 2024

Same problem here.

I agree with @Basekill 's statement:

it is a general issue with agents being initialised for you in methods (internal helper agents that are not created by the developer but in the library).

@ekzhu maybe some kind of custom_model detection strategy could be defined, perhaps detecting the presence of 'model_client_cls' in the custom model's llm_config (even though I'm not sure if this is used by oai/azure oai models too). In this way it would be possible to detect a custom model that needs registration and call register_model_client on any agent instance created within the library.

If 'model_client_cls' is already used by oai/azure oai perhaps a new key that distinguishes custom models could be required in the config.

Matteo-Frattaroli pushed a commit to Matteo-Frattaroli/autogen that referenced this issue May 15, 2024
Matteo-Frattaroli pushed a commit to Matteo-Frattaroli/autogen that referenced this issue May 15, 2024
@sonichi
Copy link
Contributor

sonichi commented May 26, 2024

There is one approach to solve the problem in a generic manner: Move the register_model_client out of the agents. Instead, make it a function to manipulate configs. Then, pass the materialized configs to agents so that the agents never need to worry about register_model_client.

@robos4n
Copy link

robos4n commented Sep 24, 2024

is there any traction on this PR 2696 which should address this bug? It's been nearly a month since the last comment.

@rysweet rysweet added 0.2 Issues which are related to the pre 0.4 codebase needs-triage labels Oct 2, 2024
@thainduy
Copy link
Contributor

thainduy commented Oct 8, 2024

i have the same problem. is there any workaround at the moment?

@marklysze
Copy link
Collaborator

Hey all, can you please check updates to #2696

ekzhu added a commit that referenced this issue Oct 11, 2024
* remove unused import statement

* fix #2643: register custom model clients within GroupChat

* add docs for fix #2643

* Update website/docs/topics/groupchat/using_custom_models.md

Co-authored-by: Chi Wang <[email protected]>

* Update website/docs/topics/groupchat/using_custom_models.md

Co-authored-by: Chi Wang <[email protected]>

* fix: removed unnecessary llm_config from checking agent

* fix: handle missing config or "config_list" key in config

* fix: code formatting

* Isolate method for internal agents creation

* Add unit test to verify that internal agents' client actually registers ModelClient class

* fix: function arguments formatting

* chore: prepend "select_speaker_auto_" to llm_config and model_client_cls attributes in GroupChat

* feat: use selector's llm_config for speaker selection agent if none is passed to GroupChat

* Update test/agentchat/test_groupchat.py

* Update groupchat.py - moved class parameters around, added to docstring

* Update groupchat.py - added selector to async select speaker functions

* Update test_groupchat.py - Corrected test cases for custom model client class

* Update test_groupchat.py pre-commit tidy

---------

Co-authored-by: Matteo Frattaroli <[email protected]>
Co-authored-by: Chi Wang <[email protected]>
Co-authored-by: Eric Zhu <[email protected]>
Co-authored-by: Mark Sze <[email protected]>
@marklysze
Copy link
Collaborator

Closing as this issue has been resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.2 Issues which are related to the pre 0.4 codebase group chat/teams group-chat-related issues help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants