koaning / memo

Decorators that logs stats.
https://koaning.github.io/memo/getting-started.html
MIT License
103 stars 9 forks source link

Add support for optional ray backend #26

Open bradday4 opened 3 years ago

bradday4 commented 3 years ago

So I did some testing and it seems as though adding support for ray should be relatively straight forward. I've only tested this locally as I don't have a remote machine to set up on right now. I really don't know how the data list will be appended to in a remote context. Something definitely worth investigating I think. To which, if you have an extra ray server laying around and don't mind sharing it I'd be more than happy to test it out.

The following will enable joblib to work with ray locally.

From the shell

ray start --head --port=6379

At the top of demo.py

import ray
ray.init(address='auto', _redis_password='5241590000000000')
...
...
runner = Runner(backend="ray", n_jobs=-1)

And at the beginning of Runner._run

  if self.backend == "ray":
      try:
          from ray.util.joblib import register_ray
          register_ray()
      except ImportError as e:
          import sys
          raise type(e)(
              str(e) + "\nRay backend must be installed"
          ).with_traceback(sys.exc_info()[2])

This should leave the memo package decoupled from ray so it's optional for the user. We could also catch the import error and default back to joblibs loky but my feeling is that if someone passes the backend as ray then failing would be better then raising a warning and continuing running locally. Your thoughts?

koaning commented 3 years ago

I don't have a ray server around, unfortunately. That said, part of me is also wondering to what extent we want to benchmark this. Having a benchmark that tells us this change is meaningful might be enough, mainly because we might need feedback from other users to get a good picture of what's important to improve/focus on.

On my end, this looks green to start working on a PR. 👍

bradday4 commented 3 years ago

Sounds good. I'll start working on a PR then. Do you want the PR to go back to Main? Or should there be a dev branch to submit PR's to. I noticed I missed deleting an unused import last time that caused a build failure on your end.

koaning commented 3 years ago

Feel free to push to main. You might want to pull first though, I cleaned up the repo with regards to docs in between your PR and this new one.