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

Implement Parallel Function Calls with List[Union[T]] #378

Merged
merged 19 commits into from
Feb 1, 2024
Merged

Conversation

jxnl
Copy link
Owner

@jxnl jxnl commented Feb 1, 2024

Ellipsis 🚀 This PR description was created by Ellipsis for commit 82991b6.

Summary:

This PR introduces parallel function calls using a list of union types, modifies the instructor's patching mechanism to handle these calls, and includes tests for the new functionality.

Key points:

  • Added PARALLEL_TOOLS to Mode enum in instructor/function_calls.py.
  • Created ParallelBase class in instructor/dsl/parallel.py to handle parallel function calls.
  • Modified instructor/patch.py to handle PARALLEL_TOOLS mode and use ParallelBase for response processing.
  • Added tests for parallel function calls in tests/openai/test_parallel.py.
  • Created an example of using parallel function calls in examples/patching/pcalls.py.

Generated with ❤️ by ellipsis.dev

@jxnl jxnl changed the title Parallel calls Implement Parallel Function Calls with List[Union[T]] Feb 1, 2024
@jxnl jxnl marked this pull request as ready for review February 1, 2024 02:08
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Changes requested.


Something look wrong? You can customize Ellipsis by editing the ellipsis.yaml for this repository.

Generated with ❤️ by ellipsis.dev

#! We expect this from the OpenAISchema class, We should address
#! this with a protocol or an abstract class... @jxnlco
assert mode == Mode.PARALLEL_TOOLS, "Mode must be PARALLEL_TOOLS"
print(response.choices[0].message)
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a print statement on line 24 which seems to be for debugging purposes. It's a good practice to remove such statements before merging the code to the main branch.

Suggested change
print(response.choices[0].message)
# print(response.choices[0].message)

@CyrusOfEden
Copy link

Would a change from Iterable[Union[…]] to tuple[…] or even just (…) be to your liking aesthetically? Happy to figure out the implementation and update this PR if you’re aligned

@jxnl
Copy link
Owner Author

jxnl commented Feb 1, 2024

Would a change from Iterable[Union[…]] to tuple[…] or even just (…) be to your liking aesthetically? Happy to figure out the implementation and update this PR if you’re aligned

its not a tuple. its Iterable[Union[T]] once it streams

@CyrusOfEden
Copy link

Would a change from Iterable[Union[…]] to tuple[…] or even just (…) be to your liking aesthetically? Happy to figure out the implementation and update this PR if you’re aligned

its not a tuple. its Iterable[Union[T]] once it streams

I agree that that's the correct type, I'm updating my suggestion to passing a list of [T1, T2, T3] communicates the intent and is more terse

@jxnl
Copy link
Owner Author

jxnl commented Feb 1, 2024

Would a change from Iterable[Union[…]] to tuple[…] or even just (…) be to your liking aesthetically? Happy to figure out the implementation and update this PR if you’re aligned

its not a tuple. its Iterable[Union[T]] once it streams

I agree that that's the correct type, I'm updating my suggestion to passing a list of [T1, T2, T3] communicates the intent and is more terse

we're going to use generics to reflect the type to the response. [T1, T2, T3] is not a typehint

resp: [T1, T2, T3] = create(response_model=[T1, T2, T3]) is not a thing

@jxnl jxnl enabled auto-merge (squash) February 1, 2024 04:03
@jxnl jxnl disabled auto-merge February 1, 2024 04:03
@jxnl jxnl merged commit 62e6a40 into main Feb 1, 2024
9 of 12 checks passed
@jxnl jxnl deleted the parallel-calls branch February 1, 2024 04:03
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.

2 participants