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

Batching #2051

Closed
wants to merge 15 commits into from
Closed

Batching #2051

wants to merge 15 commits into from

Conversation

cathyzbn
Copy link
Contributor

@cathyzbn cathyzbn commented Jul 25, 2024

Describe your changes

Enable batching in modal functions and class methods.
Interface Examples:

@app.function()
@modal.batch(batch_max_size=4, batch_linger_ms=1000)
async def batch_squared(x):
    return [x_i ** 2 for x_i in x]

@app.cls()
class Batch:
    @modal.batch(batch_max_size=4, batch_linger_ms=1000)
    async def batch_squared(self, x):
        return [x_i ** 2 for x_i in x]
Backward/forward compatibility checks

Check these boxes or delete any item (or this section) if not relevant for this PR.

  • Client+Server: this change is compatible with old servers
  • Client forward compatibility: this change ensures client can accept data intended for later versions of itself

Note on protobuf: protobuf message changes in one place may have impact to
multiple entities (client, server, worker, database). See points above.


Changelog

@cathyzbn cathyzbn marked this pull request as ready for review July 31, 2024 17:52
@cathyzbn cathyzbn requested review from mwaskom and gongy July 31, 2024 18:11
@cathyzbn
Copy link
Contributor Author

cathyzbn commented Jul 31, 2024

For parameter names, @batch and batch_max_size / batch_linger_ms may be redundant, but max_size seems unintuitive especially for ML users who are accustomed to using batch_size?

@mwaskom
Copy link
Contributor

mwaskom commented Jul 31, 2024

Does this mean we can't do batching with webhook functions?

@mwaskom
Copy link
Contributor

mwaskom commented Jul 31, 2024

@Batch and batch_max_size / batch_linger_ms may be redundant, but max_size seems unintuitive especially for ML users who are accustomed to using batch_size?

I think it should be intuitive in the context of a batch decorator with a small number of parameters. If the decorator had a lot of parameters maybe the link would be less clear.

@mwaskom
Copy link
Contributor

mwaskom commented Jul 31, 2024

Where is "linger_ms" coming from by the way? At one point I thought we had max_size and max_wait which I thought nicely contrasted the two parameters that you trade off. "linger" isn't really evoking anything specific for me relating to batching, but maybe I'm missing a reference point.

@cathyzbn
Copy link
Contributor Author

Does this mean we can't do batching with webhook functions?

Yes, we can't batch the webhook functions but we can do this. I think most of the use cases would probably involve declaring a class for model + make batched inference as one of its methods

@app.function()
@modal.batch(batch_max_size=4, batch_linger_ms=1000)
async def batched_function_async(x):
    return [x_i**2 for x_i in x]

@app.function()
@modal.web_endpoint()
async def f(x: int):
    output = await batched_function_async.remote.aio(x)
    return output

@mwaskom
Copy link
Contributor

mwaskom commented Jul 31, 2024

What's the technical reason that we can't decorate an endpoint function or method with the batching decorator? I think users will naively expect those to compose.

@gongy
Copy link
Contributor

gongy commented Jul 31, 2024

We're looking into getting it to work with web endpoints, but the short summary is that each web endpoints invocation has ASGI streams of incoming/outgoing data. It's difficult to batch four streams into a single request/response pattern unless the user defines their request handler explicitly.

@cathyzbn
Copy link
Contributor Author

cathyzbn commented Aug 5, 2024

@cathyzbn cathyzbn closed this Aug 5, 2024
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.

3 participants