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

[core][aDAG] Multi ref output doesn't work with asyncio.gather #47684

Open
rkooo567 opened this issue Sep 16, 2024 · 3 comments
Open

[core][aDAG] Multi ref output doesn't work with asyncio.gather #47684

rkooo567 opened this issue Sep 16, 2024 · 3 comments
Labels
accelerated-dag bug Something that is supposed to be working; but isn't core Issues that should be addressed in Ray Core triage Needs triage (eg: priority, bug/not-bug, and owning component)

Comments

@rkooo567
Copy link
Contributor

What happened + What you expected to happen

When we have multi output refs, we want to batch wait the result. For the sync case, I verified ray.get works. But for async case, asyncio.gather doesn't seem to work.

Versions / Dependencies

master

Reproduction script

import asyncio
import ray
from ray.dag import InputNode, MultiOutputNode

async def main():
    @ray.remote
    class A:
        def f(self, i):
            return i

    a = A.remote()
    b = A.remote()

    with InputNode() as inp:
        x = a.f.bind(inp)
        y = b.f.bind(inp)
        dag =  MultiOutputNode([x, y])

    adag = dag.experimental_compile(enable_asyncio=True)
    refs = await adag.execute_async(1)
    outputs = []
    # works
    # for ref in refs:
    #     outputs.append(await ref)
    # doesn't work.
    outputs = await asyncio.gather(*refs)
    print(outputs)

asyncio.run(main())

Issue Severity

None

@rkooo567 rkooo567 added bug Something that is supposed to be working; but isn't triage Needs triage (eg: priority, bug/not-bug, and owning component) accelerated-dag labels Sep 16, 2024
@anyscalesam anyscalesam added the core Issues that should be addressed in Ray Core label Sep 16, 2024
@jeffreyjeffreywang
Copy link
Contributor

Need to support ray.wait on CompiledDAGFuture and a list of CompiledDAGFuture as well. @rkooo567, is this blocking you from adopting the new execute_async API in vLLM? I'm trying to assess the priority of this issue.

@rkooo567
Copy link
Contributor Author

rkooo567 commented Sep 16, 2024

In vLLM we don't need it (we only await on the first ref). It is mainly for beta release (aiming the mid Oct).

Also one quick note is that we don't support ray.wait on futures in Ray (we use asyncio.gather instead).

Also other thing to mention is that ray.wait may be more useful when you have multiple dags and wait concurrently.

@jeffreyjeffreywang
Copy link
Contributor

Got it, will prioritize this over the deserialization issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accelerated-dag bug Something that is supposed to be working; but isn't core Issues that should be addressed in Ray Core triage Needs triage (eg: priority, bug/not-bug, and owning component)
Projects
None yet
Development

No branches or pull requests

3 participants