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

Parallelise with ray #113

Merged
merged 16 commits into from
Aug 17, 2022
Merged

Parallelise with ray #113

merged 16 commits into from
Aug 17, 2022

Conversation

Xuzzo
Copy link
Collaborator

@Xuzzo Xuzzo commented Aug 12, 2022

  • New interruptible worker with ray.
  • All Shapley methods are now independent of map_reduce.
  • truncated_montecarlo_shapley is the new default method
  • adds tests for truncated_montecarlo_shapley

Closes #22

@Xuzzo Xuzzo added enhancement New feature or request testing Writing and verifying tests (unit or otherwise) labels Aug 12, 2022
@Xuzzo Xuzzo self-assigned this Aug 12, 2022
tests/conftest.py Outdated Show resolved Hide resolved
@Xuzzo
Copy link
Collaborator Author

Xuzzo commented Aug 16, 2022

Sorry for the deleted type hints. Upon merging I had a bunch of conflicts and in the attempt to solve them I removed some of your stuff. Will fix it now.

@Xuzzo Xuzzo mentioned this pull request Aug 16, 2022
4 tasks
@AnesBenmerzoug
Copy link
Collaborator

One change that we could make later if we really want to have an abstract interface for the parallelization would be to switch from decorating the classes with @ray.remote to applying it in the __new__ method. i.e. change from

@ray.remote
class ShapleyWorker:
   ...

to for example:

class RayWorkerBase:
   def __new__(cls, *args, **kwargs):
       remote_cls = ray.remote(*args, **kwargs)(cls)
       actor = remote_cls.remote()
       return actor

class ShapleyWorker(RayWorkerBase):
   ...

Of course, this is just an idea

@Xuzzo
Copy link
Collaborator Author

Xuzzo commented Aug 17, 2022

I tried your suggestion (the one about RayWorkerBase), but there are some things not working properly. Will keep the decorator for now and leave the abstract interface for the future. Also, I have removed the fake remote method from the worker and coordinator and just ignored the type since it was giving me annoying warnings.

@AnesBenmerzoug
Copy link
Collaborator

Looks good to me. Feel free to merge.

@Xuzzo Xuzzo merged commit c076ea2 into develop Aug 17, 2022
@AnesBenmerzoug AnesBenmerzoug deleted the parallelise_with_ray branch October 7, 2022 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request testing Writing and verifying tests (unit or otherwise)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Ray for InterruptibleWorker
3 participants