- 
                Notifications
    You must be signed in to change notification settings 
- Fork 2.4k
Add parallel tool execution to ReAct #8999
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
base: main
Are you sure you want to change the base?
Conversation
- Modified ReAct signature to accept next_tool_calls list instead of single tool_name/tool_args - Implemented parallel execution for sync using ThreadPoolExecutor - Implemented parallel execution for async using asyncio.gather - Updated trajectory format to support multiple tool calls and observations per iteration - Added comprehensive tests for parallel tool execution (both sync and async) - Updated all existing tests to use new format - All tests passing with proper linting Co-authored-by: TomeHirata <33407409+TomeHirata@users.noreply.github.com>
- Remove unnecessary list() conversion from asyncio.gather return - Remove unused elapsed_time calculations from tests - All tests still passing with proper linting Co-authored-by: TomeHirata <33407409+TomeHirata@users.noreply.github.com>
        
          
                tests/predict/test_react_parallel.py
              
                Outdated
          
        
      | @@ -0,0 +1,374 @@ | |||
| """Tests for parallel tool execution in ReAct.""" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot can we have all tests in tests/predict/test_react.py?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! All tests have been consolidated into tests/predict/test_react.py and test_react_parallel.py has been removed. (commit 741ac11)
- Consolidated all parallel tool execution tests into test_react.py - Removed test_react_parallel.py as requested in code review - Added asyncio and time imports to support parallel tests - All 14 tests passing with proper linting Co-authored-by: TomeHirata <33407409+TomeHirata@users.noreply.github.com>
| I am worrying that the change to prompt can lead to performance regression, since it might be possible LM does a worse job selecting multiple tools in a bulk. We can run benchmarks to verify it, or probably we should introduce a new mode for parallel execution? @copilot what's your thought on this? | 
| 
 You raise a valid concern. The current implementation does change the prompt for all ReAct instances, which could impact performance in single-tool scenarios. A few thoughts on addressing this: 
 Would you like me to implement an optional mode parameter, or would you prefer to evaluate performance first? | 
Parallel Tool Execution for ReAct
This PR adds support for parallel tool execution in the ReAct module, allowing the LLM to request multiple tool calls in a single turn and execute them concurrently.
Plan
forwardmethod using ThreadPoolExecutoraforwardmethod using asyncio.gatherSummary
This implementation allows the LLM to request multiple tool calls in a single turn, which are then executed concurrently:
ThreadPoolExecutorfor parallel executionasyncio.gatherfor parallel executiontest_react.pyfor better organizationKey Changes
next_tool_name+next_tool_argstonext_tool_calls(list of dicts)tool_calls_X(list) andobservations_X(list of dicts) instead of individual fieldstest_react.pyOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.