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.

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

feat: add Terminator for agent to terminate #254

Merged
merged 23 commits into from
Oct 25, 2023
Merged

feat: add Terminator for agent to terminate #254

merged 23 commits into from
Oct 25, 2023

Conversation

zechengz
Copy link
Member

@zechengz zechengz commented Aug 21, 2023 •
edited
Loading

Description

Add Terminator triggers for agent to terminate. In this implementation I create two types of terminators,

  • One is token limit terminator, which terminates agent if token exceeds the model token limit
  • Another one is the response words terminator, which terminates the agent if some preset words appear more than or equal to the threshold

(will create another PR to update examples)

Motivation and Context

Current termination is a little bit messy, this PR partly resolves the #191

  • I have raised an issue to propose this change (required for new features and bug fixes)

Types of changes

What types of changes does your code introduce? Put an x in all the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds core functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (update in the documentation)
  • Example (update in the folder of example)

Checklist

Go over all the following points, and put an x in all the boxes that apply.
If you are unsure about any of these, don't hesitate to ask. We are here to help!

  • I have read the CONTRIBUTION guide. (required)
  • My change requires a change to the documentation.
  • I have updated the tests accordingly. (required for a bug fix or a new feature)
  • I have updated the documentation accordingly.

@zechengz zechengz added the Agent Related to camel agents label Aug 21, 2023
@zechengz zechengz self-assigned this Aug 21, 2023
lightaime
lightaime reviewed Aug 30, 2023
View reviewed changes
Copy link
Member

@lightaime lightaime left a comment

Choose a reason for hiding this comment

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

Very cool!! Left some minor comments.

camel/response/response.py Outdated Show resolved Hide resolved
camel/response/__init__.py Outdated Show resolved Hide resolved
camel/agents/chat_agent.py Outdated Show resolved Hide resolved
camel/agents/critic_agent.py Outdated Show resolved Hide resolved
camel/agents/chat_agent.py Outdated Show resolved Hide resolved
camel/termination/token_limit_termination.py Outdated Show resolved Hide resolved
camel/termination/token_limit_termination.py Outdated Show resolved Hide resolved
camel/termination/response_termination.py Outdated Show resolved Hide resolved
camel/termination/response_termination.py Outdated Show resolved Hide resolved
camel/agents/chat_agent.py Outdated Show resolved Hide resolved
@zechengz zechengz changed the title Add Termination for agent to terminate Add Terminator for agent to terminate Sep 9, 2023
@zechengz zechengz marked this pull request as ready for review September 10, 2023 08:40
@Obs01ete Obs01ete self-requested a review September 10, 2023 15:05
Obs01ete
Obs01ete requested changes Sep 11, 2023
View reviewed changes
Copy link
Collaborator

@Obs01ete Obs01ete 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 fine. Looks like overengineering, but let it be. What about the user side code? Do we still have all the excessive code as mentioned in #191? Simplifying the user code would be of the biggest help.

camel/agents/chat_agent.py Outdated Show resolved Hide resolved
camel/agents/chat_agent.py Outdated Show resolved Hide resolved
camel/agents/chat_agent.py Show resolved Hide resolved
@Obs01ete Obs01ete mentioned this pull request Sep 13, 2023
Open
14 tasks
@Benjamin-eecs Benjamin-eecs changed the title Add Terminator for agent to terminate feat: add Terminator for agent to terminate Sep 18, 2023
@Obs01ete
Copy link
Collaborator

@zechengz any plans to address the comments in the review?

@zechengz
Copy link
Member Author

@Obs01ete Thanks for the reminding. I will probably address the comments tomorrow or the day after tomorrow.

@zechengz
Copy link
Member Author

@Obs01ete For now there is about 1 line of code if user specifies the response word terminator (https://github.com/camel-ai/camel/pull/254/files#diff-a282cc20e68a563a489018ae629fb301a7e54929683c68d270dbdcb0c249c4ccR307) IMO it should be ok.

Obs01ete
Obs01ete reviewed Oct 19, 2023
View reviewed changes
Copy link
Collaborator

@Obs01ete Obs01ete left a comment

Choose a reason for hiding this comment

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

The concept looks reasonable but I do not comprehend how this code works. If you are sure that it works, and it will not destroy the user experience, then it is fine.
@lightaime The amount of complexity added is quite high. Does the development come easier with this change? What about the risk of bugs here?

camel/agents/chat_agent.py Outdated Show resolved Hide resolved
camel/agents/chat_agent.py Outdated Show resolved Hide resolved
lightaime
lightaime approved these changes Oct 19, 2023
View reviewed changes
Copy link
Member

@lightaime lightaime left a comment

Choose a reason for hiding this comment

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

Thanks @zechengz! LGTM! Just a couple nit comments.

camel/agents/chat_agent.py Outdated Show resolved Hide resolved
camel/agents/chat_agent.py Outdated Show resolved Hide resolved
camel/agents/chat_agent.py Outdated Show resolved Hide resolved
camel/terminator/__init__.py Outdated Show resolved Hide resolved
@lightaime
Copy link
Member

Awesome. Thanks @zechengz!

@lightaime lightaime merged commit 8ace5e4 into master Oct 25, 2023
8 checks passed
@lightaime lightaime deleted the zecheng_add_some_termination branch October 25, 2023 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lightaime lightaime lightaime approved these changes

@Obs01ete Obs01ete Awaiting requested review from Obs01ete

Assignees

@zechengz zechengz

Labels
Agent Related to camel agents
Projects
Project Camel
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zechengz @Obs01ete @lightaime