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

Register async human input handler #794

Merged
merged 11 commits into from
Dec 3, 2023
Merged

Conversation

ShobhitVishnoi30
Copy link
Collaborator

@ShobhitVishnoi30 ShobhitVishnoi30 commented Nov 28, 2023

Summary

This PR registers the async version of the human input handler, a_check_termination_and_human_reply, in addition to the sync version. This allows the async a_get_human_input method to be leveraged during async reply generation.

Why are these changes needed?

Without this change, only the sync human input method would get called during async replies. By registering the async handler, we enable async human input when needed.

Changes

  • Added self.register_reply() call in __init__ to register ConversableAgent.a_check_termination_and_human_reply

Related issue number

closes 806

Checks

@ShobhitVishnoi30
Copy link
Collaborator Author

@sonichi @afourney @pcdeadeasy @qingyun-wu @rickyloynd-microsoft can you guys review it.I am not able to add reviewers

@qingyun-wu
Copy link
Contributor

Thanks for the contribution! Is it possible to add a notebook example to demonstrate the usage and utility of this async human reply?

@ShobhitVishnoi30
Copy link
Collaborator Author

Thanks for the contribution! Is it possible to add a notebook example to demonstrate the usage and utility of this async human reply?

yes sure let me do that

@ShobhitVishnoi30
Copy link
Collaborator Author

Please resolve my comment and fix the code format error. You can use pre-commit to do that.

formatting is done

Copy link
Contributor

@sonichi sonichi left a comment

Choose a reason for hiding this comment

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

@qingyun-wu should this notebook be added to the example page?

@sonichi sonichi added this pull request to the merge queue Dec 3, 2023
Merged via the queue into microsoft:main with commit 5c92fb3 Dec 3, 2023
76 of 83 checks passed
@momuno
Copy link
Member

momuno commented Dec 3, 2023

Hi @sonichi, I just left a comment on this commit. I'm wondering if this PR is a breaking change for other agents in contrib/ that have taken a dependency on the prior register reply order.
Commit comment here:
https://github.com/microsoft/autogen/commit/5c92fb378b4c5db86f020bc15891b30014abb5f7#commitcomment-134094264

@sonichi
Copy link
Contributor

sonichi commented Dec 3, 2023

Hi @sonichi, I just left a comment on this commit. I'm wondering if this PR is a breaking change for other agents in contrib/ that have taken a dependency on the prior register reply order. Commit comment here: https://github.com/microsoft/autogen/commit/5c92fb378b4c5db86f020bc15891b30014abb5f7#commitcomment-134094264

I can't see the commit comment. But you are right, teachable agent and retrieve user proxy agent need to be updated.

@ShobhitVishnoi30
Copy link
Collaborator Author

agent

register reply with position argument is being used in text_analyzer_agent , teachable_agent and math_user_proxy_agent.

Do we need to update it to N+1 ??

@momuno
Copy link
Member

momuno commented Dec 4, 2023

Hey thanks for the confirmation. I can help with a PR later today if still needed.

momuno pushed a commit to momuno/autogen that referenced this pull request Dec 5, 2023
github-merge-queue bot pushed a commit that referenced this pull request Dec 6, 2023
…trib/ agents per PR #794 (#871)

Co-authored-by: Mollie Munoz <momuno@microsoft.com>
rlam3 pushed a commit to rlam3/autogen that referenced this pull request Dec 19, 2023
* Update conversable_agent.py

* Add files via upload

* Delete notebook/Async_human_input.ipynb

* Add files via upload

* refactor:formatter

* feat:updated position

---------

Co-authored-by: Chi Wang <wang.chi@microsoft.com>
rlam3 pushed a commit to rlam3/autogen that referenced this pull request Dec 19, 2023
…trib/ agents per PR microsoft#794 (microsoft#871)

Co-authored-by: Mollie Munoz <momuno@microsoft.com>
whiskyboy pushed a commit to whiskyboy/autogen that referenced this pull request Apr 17, 2024
* Update conversable_agent.py

* Add files via upload

* Delete notebook/Async_human_input.ipynb

* Add files via upload

* refactor:formatter

* feat:updated position

---------

Co-authored-by: Chi Wang <wang.chi@microsoft.com>
whiskyboy pushed a commit to whiskyboy/autogen that referenced this pull request Apr 17, 2024
…trib/ agents per PR microsoft#794 (microsoft#871)

Co-authored-by: Mollie Munoz <momuno@microsoft.com>
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

Successfully merging this pull request may close these issues.

Registering Async Reply Enabled Async Human Input
5 participants