autogen icon indicating copy to clipboard operation
autogen copied to clipboard

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

Open krlng opened this issue 1 year ago • 7 comments

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

krlng avatar Jan 11 '24 20:01 krlng

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

rickyloynd-microsoft avatar Jan 11 '24 22:01 rickyloynd-microsoft

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?

ekzhu avatar Jan 12 '24 07:01 ekzhu

@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?

krlng avatar Jan 12 '24 08:01 krlng

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: https://github.com/microsoft/autogen/blob/2e519b016a8bfa7a807721a1fc8a93b5d3be6c32/autogen/agentchat/conversable_agent.py#L185-L190

davorrunje avatar Jan 12 '24 08:01 davorrunje

@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 avatar Jan 12 '24 08:01 davorrunje

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:

https://github.com/microsoft/autogen/blob/11ebd3c42aa4c8a5e2b8a3fdaf36be15510f29a6/test/agentchat/test_conversable_agent.py#L93-L107

davorrunje avatar Jan 12 '24 10:01 davorrunje

The example you posted looks nice 👍 The wished additional issue: https://github.com/microsoft/autogen/issues/1233

krlng avatar Jan 13 '24 21:01 krlng