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

Re-enable recursive remote functions in a limited form. #453

Merged
merged 2 commits into from
Apr 13, 2017
Merged

Re-enable recursive remote functions in a limited form. #453

merged 2 commits into from
Apr 13, 2017

Conversation

robertnishihara
Copy link
Collaborator

This should partially address #398. This worked before but was broken because it wasn't being tested in the CI.

However, it's unclear if we should support this because we currently cannot fully support it.

For example, the following works.

@ray.remote
def f():
  return [f.remote()]

f.remote()

However, the following does not work.

def create_f():
  @ray.remote
  def f():
    return [f.remote()]
  return f

create_f().remote()

The following also does not work.

def create_and_call_f():
  @ray.remote
  def f():
    return [f.remote()]

  f.remote()  

create_and_call_f()

Given that we aren't supporting it fully, do we want to partially support it?

@mehrdadn what do you think about this?

@AmplabJenkins
Copy link

Merged build finished. Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/562/
Test PASSed.

@AmplabJenkins
Copy link

Merged build finished. Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/563/
Test PASSed.

@mehrdadn
Copy link
Contributor

mehrdadn commented Apr 13, 2017

Given that we aren't supporting it fully, do we want to partially support it?

Definitely yes from a user standpoint. It's not going to make anyone's life easier if you don't support it, and it definitely makes people's life easier if it's partially supported. So I would just warn the user that the functionality is incomplete and may or may not ever work perfectly (almost certainly the latter).

@pcmoritz pcmoritz merged commit c802e51 into ray-project:master Apr 13, 2017
@pcmoritz pcmoritz deleted the fixrecursion branch April 13, 2017 08:47
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.

4 participants