adrn / schwimmbad

A common interface to processing pools.
MIT License
115 stars 18 forks source link

Feature/dill serialization #25

Closed ajshajib closed 5 years ago

ajshajib commented 5 years ago

Add option for dill serialization to MPIPool. This is necessary for some softwares, e.g., dynesty.

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at ?% when pulling 6bcc32176cc42c311df34e39f55093a0f20a0727 on ajshajib:feature/dill_serialization into e00242bd48b2726e625713040aa1358d22a91d91 on adrn:master.

adrn commented 5 years ago

Thanks @ajshajib!

Could you add a test of this?

  1. Add dill to the list of packages installed by travis: https://github.com/adrn/schwimmbad/blob/master/.travis.yml#L44

  2. Add a second test here (and call it in the if __name__ == '__main__' block) to test using the new option? https://github.com/adrn/schwimmbad/blob/master/schwimmbad/tests/test_mpi.py

ajshajib commented 5 years ago

@adrn I have added a separate test function for MPIPool(use_dill=True). However, I had to put it in a separate file test_mpi_with_dill.py for the tests to finish running on Travis. I couldn't figure out why the tests were getting stuck when I put the new test function in test_mpi.py. Although, the new test is being skipped for the same reason test_mpi() is being skipped.

adrn commented 5 years ago

Strange! It looks like the test runs fine locally with:

mpiexec -n 2 python schwimmbad/tests/test_mpi_with_dill.py

I pushed a commit that adds this to the travis run, and I moved the dill import so that it is only required if use_dill=True.

adrn commented 5 years ago

Looks like things are passing - thanks @ajshajib!

adrn commented 5 years ago

@ajshajib PS: you said this is needed for dynesty. Would it help to release a new version of schwimmbad with this, or are you happy installing it from master?

ajshajib commented 5 years ago

@adrn Thanks for accepting the PR and making additional changes. While I am fine to install it from master, it would be great if you can release a new version in case some other people would need it. However, it's up to your discretion, for example, if you want to wait for some other developments in progress before bumping up the version.