-
Notifications
You must be signed in to change notification settings - Fork 399
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
Vectorize function calls #120
Conversation
52bec7b
to
3927dce
Compare
3efcda0
to
8f0797b
Compare
6fd21cb
to
cf981d6
Compare
0c2deee
to
6d41c13
Compare
Update: Vectorization is currently implemented, although it needs more documentation and tests to make the PR mergeable. |
8fa19c2
to
3d5d5c7
Compare
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.
Looks like we could use some tests for OpenAI, but I imagine that's not particularly straightforward.
Aside from that, just some questions/comments; otherwise, looks good to me.
loop = asyncio.new_event_loop() | ||
try: | ||
outputs = loop.run_until_complete(self.func()) | ||
finally: | ||
loop.close() |
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.
Do we want to provide an interface that allows the caller to manage the event loop (e.g. for use in external async applications)?
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.
I think the function that returns the new sequence should be async, but I didn't want to break the API just yet.
loop = asyncio.new_event_loop() | ||
try: | ||
outputs = loop.run_until_complete(create_and_gather_tasks()) | ||
finally: | ||
loop.close() |
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.
Same consideration here.
We need to refactor the code to make it easier to test the code that relies on the OpenAI API. That's the thing that annoys me the most currently. |
We might want a slightly different behavior for HuggingFace models: in this case we're better off flattening the input arrays so inference can be run in batch. This does not make this PR any less useful, but probably means that we'll need a different mechanism for these models. Will open an issue once this is merged. |
3d5d5c7
to
b26ffc0
Compare
b26ffc0
to
9d863e7
Compare
In this PR I introduce a
outlines.vectorize
function that works similarly tonumpy.vectorize
, and allows to turn functions into functions that take arrays as an input. The major differences withnumpy.vectorize
are:This function will be used internally for model calls, so we can pass arrays of arbitrary shapes as inputs and get an array of answers. Usual broadcasting rules apply:
I made the choice here to execute the coroutines in a new event loop before returning. This allows to keep the synchronous user-facing API for now. Of course, this restricts the potential speedups we get by using
asyncio
and we will eventually need to move towards an explicitly async API. But that would be too big of a change for a single PR.vectorize
vectorize
for HF model calls