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] aDAG silently errors if task bind args do not match function signature #47709

Open
stephanie-wang opened this issue Sep 17, 2024 · 6 comments · May be fixed by #47773
Open

[core][adag] aDAG silently errors if task bind args do not match function signature #47709

stephanie-wang opened this issue Sep 17, 2024 · 6 comments · May be fixed by #47773
Assignees
Labels
bug Something that is supposed to be working; but isn't compiled-graph core Issues that should be addressed in Ray Core needs-repro-script Issue needs a runnable script to be reproduced P1 Issue that should be fixed within a few weeks

Comments

@stephanie-wang
Copy link
Contributor

What happened + What you expected to happen

We should throw an error if a task's bind args do not match its function signature.

Versions / Dependencies

3.0dev

Reproduction script

@ray.remote
class Actor:
  def foo(self):
    # doesn't take any args.
    return

a = Actor.remote()
with ray.dag.InputNode() as inp:
  # this line should error.
  dag = a.foo.bind(inp)

Issue Severity

None

@stephanie-wang stephanie-wang added bug Something that is supposed to be working; but isn't P1 Issue that should be fixed within a few weeks triage Needs triage (eg: priority, bug/not-bug, and owning component) compiled-graph labels Sep 17, 2024
@kevin85421
Copy link
Member

kevin85421 commented Sep 20, 2024

I can't reproduce this issue. The exception is thrown when execute is called. Or should the exception be thrown when bind is called?

import ray
from ray.dag import InputNode


@ray.remote
class Actor:
  def foo(self):
    # doesn't take any args.
    return

a = Actor.remote()
with InputNode() as inp:
  # this line should error.
  dag = a.foo.bind(inp)
compiled_dag = dag.experimental_compile()

ref = dag.execute(5)
print(ray.get(ref))
image

@rkooo567
Copy link
Contributor

I think the ideal case is .bind raise an exception (because that's easiest to debug)

a.foo.bind(inp) requires a single input, but the function `Actor.foo` doesn't accept any argument 

@kevin85421 kevin85421 self-assigned this Sep 20, 2024
@stephanie-wang
Copy link
Contributor Author

Hmm @woshiyyya can you comment on your use case? I haven't tried to reproduce yet but I thought you ran into this bug earlier.

@rkooo567
Copy link
Contributor

@stephanie-wang i actually already have a pr out to fix it I think

@stephanie-wang stephanie-wang added the needs-repro-script Issue needs a runnable script to be reproduced label Sep 22, 2024
@anyscalesam anyscalesam added the core Issues that should be addressed in Ray Core label Sep 24, 2024
@stephanie-wang
Copy link
Contributor Author

@stephanie-wang i actually already have a pr out to fix it I think

Can you link to the PR and assign yourself? If not, this is a good first issue.

@rkooo567
Copy link
Contributor

rkooo567 commented Oct 3, 2024

done!

@rkooo567 rkooo567 removed the triage Needs triage (eg: priority, bug/not-bug, and owning component) label Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that is supposed to be working; but isn't compiled-graph core Issues that should be addressed in Ray Core needs-repro-script Issue needs a runnable script to be reproduced P1 Issue that should be fixed within a few weeks
Projects
None yet
4 participants