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

feat: react agent integration #179

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Conversation

HalberdOfPineapple
Copy link
Member

@HalberdOfPineapple HalberdOfPineapple commented Jun 22, 2023

Description

Basically implemented the agent which outputs in ReAct pattern but only tested on several QA tasks. This PR is mainly for review purposes.

Motivation and Context

Why is this change required? What problem does it solve?
If it fixes an open issue, please link to the issue here.
You can use the syntax close #15213 if this solves the issue #15213

  • 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)

Implemented Tasks

  • Subtask 1
  • Subtask 2
  • Subtask 3

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.

The file structure in master branch was modified to be different as this
branch. The merge is for convenience of developing later.
@lightaime lightaime added the Agent Related to camel agents label Jun 30, 2023
@Benjamin-eecs Benjamin-eecs changed the title Feature/re act feat: react agent integration Jun 30, 2023
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.

Overall looks good. Tests must be added.

@@ -0,0 +1,222 @@
# =========== Copyright 2023 @ CAMEL-AI.org. All Rights Reserved. ===========
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rename to lower case react_agent.py

self,
input_message: ChatMessage,
max_turns: int = 10,
) -> Tuple[ChatMessage, bool, Dict[str, Any]]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

The signature of step does not follow the signature of the base class. If you want to introduce a new signature you must name the method differently

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry but this pattern was borrowed fromEmbodiedAgent and ChatAgent where they both used a completely different signature as the one in base class (def step(self) -> None:). I think the version in the base (BaseAgent) is too abstract and does not return anything and should not be used?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please do not use EmbodiedAgent as a reference. There are some deficiencies in the code that we will fix soon. Please check your code with mypy and make sure you code does not introduce any warnings. Soon mypy will be made obligatory.

Copy link
Member

@lightaime lightaime Jul 1, 2023

Choose a reason for hiding this comment

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

I do not totally agree with naming all the methods differently which will make it very difficult for the user to use. Just like PyTorch, every deviated Module has a different forward method with a different signature. It is still ok. IMO, the base class method's signature could be def step(self, *args, **kwargs) -> Any:.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The key feature of Module is polymorphism, a Module can be composed, the implementation can be abstracted away. In camel I do not see a single instance of polymorphic use of step() and reset(), implying it is not needed so far.

return action, action_input


class ReActAgent(ChatAgent):
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the nearest future we are going to move away from inheritance to a "part of" principle for agents. A chat agent must be a field of ReactAgent.

*args: Any,
) -> None:
try:
from langchain.utilities import SerpAPIWrapper
Copy link
Collaborator

Choose a reason for hiding this comment

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

You are bringing in langchain as a dependency which is not reflected in requirements.txt

Copy link
Collaborator

Choose a reason for hiding this comment

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

In pyproject.toml, it is

from camel.typing import RoleType, ModelType


def main():
Copy link
Collaborator

Choose a reason for hiding this comment

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

Every example must be tested with pytest, parametrized with at least ModelType.STUB



class ReActAgent(ChatAgent):
r"""Class for managing conversions of a CAMEL agent following ReAct pattern.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Every introduced code must be covered by a test.

from camel.agents.tool_agents import BaseToolAgent


class GoogleToolAgent(BaseToolAgent):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Every introduced code must be covered by a test.

return p.encode().decode("unicode-escape").encode("latin1").decode("utf-8")


class WikiToolAgent(BaseToolAgent):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Every introduced code must be covered by a test.

@HalberdOfPineapple
Copy link
Member Author

HalberdOfPineapple commented Jul 1, 2023

Overall looks good. Tests must be added.

Thanks very much for these comments. I know there are many problems with this commit and because I spent past several days working on another functionality (OpenAI function calling) I have not updated them on time. I will continue on this following your guidance later :)

@HalberdOfPineapple
Copy link
Member Author

Hi, currently I ran into some problems:

  1. the test cannot be passed because the package langchain is not installed when Github does the check, which leads to failure of examples with Google search included. But I did included langchain in requirements.txt
  2. For integrating Google search, some external API is needed and here I used the wrapper of SerpAPI (the wrapper is implemented by langchain but I can replace the wrapper later). No matter what the wrapper is, the API itself needs an API key. I am wondering whether our group can provide this for search-related functionalities (not necessarily SerpAPI) and integrate the API key in Git checks (without this my implementation cannot pass the tests because it cannot Google search operations when needed) .

@Obs01ete
Copy link
Collaborator

Obs01ete commented Jul 6, 2023

For extra packages, put them in pyproject.toml poetry section.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Agent Related to camel agents
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants