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

Expanded speaker name matching during speaker selection #2222

Closed

Conversation

marklysze
Copy link
Collaborator

@marklysze marklysze commented Mar 31, 2024

Why are these changes needed?

During the speaker selection process, the LLM returns the name of the next speaker. This is fairly reliable with OpenAI's models but with open-source/weight models they struggle with returning just the single agent name and sticking to the format of the name.

In particular two issues are:

  1. If the name has underscores some models, particularly Mistral AI's Mistral/Mixtral models, will escape them, e.g. 'Software_Developer' will be returned as 'Software\_Developer'.
  2. If the name has underscores some models will replace the underscores with spaces, e.g. 'Software_Developer' will be returned as 'Software Developer'.

In the proposed changes: GroupChat's_mentioned_agents function, agent names will now match under any of the following conditions (all continue to be case-sensitive):

  • [Unchanged] Exact name match
  • [New] If the agent name has underscores it will match with spaces instead (e.g. 'Story_writer' == 'Story writer')
  • [New] If the agent name has underscores it will match with \_ (note: single backslash) instead of _ (e.g. 'Story_writer' == 'Story\_writer')

I've added a test function to test_groupchat.py.

If we proceed with this, I'd like to add to the tips for non-OpenAI models documentation as part of this PR.

Please see #1746 which outlines a number of issues with open-source/weight models and this is a part of addressing them. I don't believe this will affect the speaker selection process when using OpenAI's models.

My plan is to continue to create PRs to address these shortcomings and then close off #1746. If it is preferred that I consolidate them into one PR, please let me know.

Related issue number

Based on shortcomings identified in #1746.

Checks

@codecov-commenter
Copy link

codecov-commenter commented Mar 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.71%. Comparing base (989c182) to head (ce6505e).

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2222       +/-   ##
===========================================
+ Coverage   37.83%   49.71%   +11.88%     
===========================================
  Files          77       77               
  Lines        7766     7766               
  Branches     1663     1800      +137     
===========================================
+ Hits         2938     3861      +923     
+ Misses       4579     3583      -996     
- Partials      249      322       +73     
Flag Coverage Δ
unittest 14.35% <ø> (?)
unittests 48.67% <ø> (+10.85%) ⬆️

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 added group chat group-chat-related issues alt-models Pertains to using alternate, non-GPT, models (e.g., local models, llama, etc.) labels Apr 1, 2024
@sonichi sonichi requested a review from olgavrou April 1, 2024 04:45
Copy link
Member

@afourney afourney left a comment

Choose a reason for hiding this comment

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

LGTM

@marklysze
Copy link
Collaborator Author

Let me know if you want me to create a new PR with a branch in the AutoGen repository for testing, rather than my own.

@sonichi
Copy link
Contributor

sonichi commented Apr 3, 2024

Let me know if you want me to create a new PR with a branch in the AutoGen repository for testing, rather than my own.

Yes, please do that and resolve the conflict.

@marklysze
Copy link
Collaborator Author

marklysze commented Apr 3, 2024

Let me know if you want me to create a new PR with a branch in the AutoGen repository for testing, rather than my own.

Yes, please do that and resolve the conflict.

No problem, all done and this can be closed.

New PR: #2267

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alt-models Pertains to using alternate, non-GPT, models (e.g., local models, llama, etc.) group chat group-chat-related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants