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

[Feature Request]: Use self instaed of class functions in register_reply #1209

Closed
krlng opened this issue Jan 11, 2024 · 7 comments
Closed

Comments

@krlng
Copy link

krlng commented Jan 11, 2024

Is your feature request related to a problem? Please describe.

I adapt the ConversableAgent using inheritance to adapt some functionalities.
Right now, I want to fix the out_of_context issue (see # TODO: #1143 handle token limit exceeded error in ConversableAgent.py).

How ever, overwriting the generate_oai_reply in my class does not have any effect if I don't overwrite the init as well, because it gets registered using the class function:
self.register_reply([Agent, None], ConversableAgent.generate_oai_reply)

My current workaround is overwriting it after calling super:

class CustomAgent(ConversableAgent):
    def __init__(
        self,
        name: str,
        system_message: Optional[str] = "You are a helpful AI Assistant.",
        description_to_group: str | None = None,
        is_termination_msg: Optional[Callable[[Dict], bool]] = None,
        max_consecutive_auto_reply: Optional[int] = None,
        human_input_mode: Optional[str] = "TERMINATE",
        function_map: Optional[Dict[str, Callable]] = None,
        code_execution_config: Optional[Union[Dict, Literal[False]]] = None,
        llm_config: Optional[Union[Dict, Literal[False]]] = None,
        default_auto_reply: Optional[Union[str, Dict, None]] = "",
    ):
        self.description_to_group = description_to_group or system_message.split("\n")[0]
        super().__init__(
            name,
            system_message,
            is_termination_msg,
            max_consecutive_auto_reply,
            human_input_mode,
            function_map,
            code_execution_config,
            llm_config,
            default_auto_reply,
        )
        ind = [i for i, x in enumerate(self._reply_func_list) if hasattr(x["reply_func"],"__name__") and x["reply_func"].__name__ == "generate_oai_reply"]
        assert len(ind) == 1
        for i in ind:
            self._reply_func_list.pop(i)
            self.register_reply([Agent, None], self.generate_oai_reply, position=i)

Describe the solution you'd like

I think it would be possible to register function calls like this:

self.register_reply([Agent, None], self.generate_oai_reply)

This would require some more changes when functions are called, i.e. removing the self in:

final, reply = reply_func(self, messages=messages, sender=sender, config=reply_func_tuple["config"])

happy to provide a pull request if wished.

Additional context

No response

@rickyloynd-microsoft
Copy link
Contributor

Instead of overriding generate_oai_reply, you class can create its own reply function and then call self.register_reply(...) on it. Here's an example: https://github.com/microsoft/autogen/blob/main/autogen/agentchat/contrib/retrieve_assistant_agent.py

@ekzhu
Copy link
Collaborator

ekzhu commented Jan 12, 2024

Maybe another solution would be to have the agent object itself as part of the argument in the custom reply function. Because often time we need to have access to the agent's state in the custom reply function, making this explicit could be a good idea. It would be easy to do, we can allow both stateless and stateful functions to be registered as reply function.

@davorrunje what do you think?

@krlng
Copy link
Author

krlng commented Jan 12, 2024

@rickyloynd-microsoft Thanks for the example. It looks pretty similar to mine, it just does not remove the entry for the old generate_oai_reply first. I guess it does not matter, as long as I return final=True. However it feels kind of wrong to have the old function I want to replace still in the list. Is there a reason I should not remove the old function from the list?

@davorrunje
Copy link
Collaborator

Maybe another solution would be to have the agent object itself as part of the argument in the custom reply function. Because often time we need to have access to the agent's state in the custom reply function, making this explicit could be a good idea. It would be easy to do, we can allow both stateless and stateful functions to be registered as reply function.

@davorrunje what do you think?

Yes, I think that was the intended purpose of the recipient parameter in the signature of the reply function:

def reply_func(
recipient: ConversableAgent,
messages: Optional[List[Dict]] = None,
sender: Optional[Agent] = None,
config: Optional[Any] = None,
) -> Tuple[bool, Union[str, Dict, None]]:

@davorrunje
Copy link
Collaborator

davorrunje commented Jan 12, 2024

@rickyloynd-microsoft Thanks for the example. It looks pretty similar to mine, it just does not remove the entry for the old generate_oai_reply first. I guess it does not matter, as long as I return final=True. However it feels kind of wrong to have the old function I want to replace still in the list. Is there a reason I should not remove the old function from the list?

Yes, we could support removing functions from the list. I am working on this part of the code right now and could include that change (#1208 ). Can you please create a new issue for it and I'll take it from there?

@davorrunje
Copy link
Collaborator

I am working on a #1215 trying to unify API for registering functions, here is an example from a test case how to use it:

def test_register_dot_for_reply() -> None:
agent = ConversableAgent("a0", max_consecutive_auto_reply=0, llm_config=False, human_input_mode="NEVER")
agent1 = ConversableAgent("a1", max_consecutive_auto_reply=0, llm_config=False, human_input_mode="NEVER")
@agent.register.for_reply(trigger=agent1)
def reply_function(
recipient: ConversableAgent,
messages: Optional[List[Dict[str, Any]]],
sender: Optional[Agent],
config: Optional[Any],
) -> Tuple[bool, Optional[Union[str, Dict[str, Any]]]]:
return (True, "hello")
agent1.initiate_chat(agent, message="hi")
assert agent1.last_message(agent)["content"] == "hello"

@krlng
Copy link
Author

krlng commented Jan 13, 2024

The example you posted looks nice 👍 The wished additional issue: #1233

whiskyboy pushed a commit to whiskyboy/autogen that referenced this issue Apr 17, 2024
* admin takeover in group chat

* comments

* add comments
@gagb gagb closed this as completed Aug 27, 2024
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

No branches or pull requests

5 participants