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

Make ActorHandles pickleable, also make proper ActorHandle and ActorC… #2007

Merged
merged 8 commits into from
May 9, 2018
Merged

Make ActorHandles pickleable, also make proper ActorHandle and ActorC… #2007

merged 8 commits into from
May 9, 2018

Conversation

robertnishihara
Copy link
Collaborator

@robertnishihara robertnishihara commented May 6, 2018

…lass classes.

This is intended to cleanup some of the Python actor code.

Note that should enable named actors via #1424 (comment).

# Record that this actor has been removed so that if this node
# dies later, the actor won't be recreated. Alternatively, we could
# remove the actor key from Redis here.
ray.worker.global_worker.redis_client.hset(b"Actor:" + actor_id,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this key in Redis used anywhere?

@AmplabJenkins
Copy link

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

@AmplabJenkins
Copy link

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

@AmplabJenkins
Copy link

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

@AmplabJenkins
Copy link

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

@robertnishihara robertnishihara changed the title [WIP] Make ActorHandles pickleable, also make proper ActorHandle and ActorC… Make ActorHandles pickleable, also make proper ActorHandle and ActorC… May 7, 2018
@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/5261/
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/5262/
Test PASSed.

}

if ray_forking:
state["forked_actor_handle_id"] = compute_actor_handle_id(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using this method to compute actor handle IDs for "forked" actor handles. However, if a user pickles and unpickles the actor handle, then I'm generating the actor handle ID randomly.

@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/5264/
Test PASSed.

@robertnishihara
Copy link
Collaborator Author

Maybe the right way to do actor handle ID generation (for pickled handles) is the following. Suppose we have

actor_handle = Actor.remote()

# Defining this remote function, which closes over actor_handle, will
# cause actor_handle to be pickled. The remote function will be broadcast
# to all the workers and unpickled there.
@ray.remote
def f():
    actor_handle.method.remote()

f.remote()

Instead of generating a random actor handle ID on each worker when the remote function is broadcast, maybe we should instead leave the actor handle in an "uninitialized" state, and then whenever a f.remote() task executes, we generate a new actor handle ID deterministically from the actor ID and the current task ID. In other words, we want it to be roughly equivalent to calling f.remote(actor_handle). Doing this will cause a proliferation of actor handles, so we'll need to make sure that is handled efficiently.

cc @stephanie-wang

@stephanie-wang
Copy link
Contributor

@robertnishihara, I like this suggestion. Just to clarify, what is the use case for closing over the actor handle vs. passing the actor handle into the f.remote() call?

@robertnishihara
Copy link
Collaborator Author

@stephanie-wang one use case is that you want to use the actor handle on a different driver (e.g., "named actors".

Another use case, I think, in RLlib/Tune is that certain user-defined functions have to have pre-specified signatures and the user isn't really able to just pass in an actor handle because that would require changing the signature and piping it through a bunch of RLlib abstractions, but they can define a function that just closes over the actor handle.

@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/5277/
Test PASSed.

@pcmoritz pcmoritz merged commit 77c8aa7 into ray-project:master May 9, 2018
@pcmoritz pcmoritz deleted the serializeactorhandles branch May 9, 2018 02:19
alok added a commit to alok/ray that referenced this pull request May 9, 2018
* master: (25 commits)
  [DataFrame] Add direct pandas imports for MVP (ray-project#1960)
  Make ActorHandles pickleable, also make proper ActorHandle and ActorC… (ray-project#2007)
  Expand local_dir in Trial init (ray-project#2013)
  Fixing ascii error for Python2 (ray-project#2009)
  [DataFrame] Implements df.update (ray-project#1997)
  [DataFrame] Implements df.as_matrix (ray-project#2001)
  [DataFrame] Implement quantile (ray-project#1992)
  [DataFrame] Impement sort_values and sort_index (ray-project#1977)
  [DataFrame] Implement rank (ray-project#1991)
  [DataFrame] Implemented prod, product, added test suite (ray-project#1994)
  [DataFrame] Implemented __setitem__, select_dtypes, and astype (ray-project#1941)
  [DataFrame] Implement diff (ray-project#1996)
  [DataFrame] Implemented nunique, skew (ray-project#1995)
  [DataFrame] Implements filter and dropna (ray-project#1959)
  [DataFrame] Implements df.pipe (ray-project#1999)
  [DataFrame] Apply() for Lists and Dicts (ray-project#1973)
  Clean up syntax for supported Python versions. (ray-project#1963)
  [DataFrame] Implements mode, to_datetime, and get_dummies (ray-project#1956)
  [DataFrame] Fix dtypes (ray-project#1930)
  keep_dims -> keepdims (ray-project#1980)
  ...
alok added a commit to alok/ray that referenced this pull request May 9, 2018
* master:
  [DataFrame] Add direct pandas imports for MVP (ray-project#1960)
  Make ActorHandles pickleable, also make proper ActorHandle and ActorC… (ray-project#2007)
  Expand local_dir in Trial init (ray-project#2013)
  Fixing ascii error for Python2 (ray-project#2009)
  [DataFrame] Implements df.update (ray-project#1997)
  [DataFrame] Implements df.as_matrix (ray-project#2001)
  [DataFrame] Implement quantile (ray-project#1992)
  [DataFrame] Impement sort_values and sort_index (ray-project#1977)
  [DataFrame] Implement rank (ray-project#1991)
  [DataFrame] Implemented prod, product, added test suite (ray-project#1994)
  [DataFrame] Implemented __setitem__, select_dtypes, and astype (ray-project#1941)
  [DataFrame] Implement diff (ray-project#1996)
  [DataFrame] Implemented nunique, skew (ray-project#1995)
  [DataFrame] Implements filter and dropna (ray-project#1959)
  [DataFrame] Implements df.pipe (ray-project#1999)
  [DataFrame] Apply() for Lists and Dicts (ray-project#1973)
alok added a commit to alok/ray that referenced this pull request May 11, 2018
* master:
  [DataFrame] Implement where (ray-project#1989)
  [DataFrame] Add direct pandas imports for MVP (ray-project#1960)
  Make ActorHandles pickleable, also make proper ActorHandle and ActorC… (ray-project#2007)
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