-
Notifications
You must be signed in to change notification settings - Fork 95
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
Feature/pso #160
Feature/pso #160
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I'll add some doc strings later. Let me know when you want to approve the merge!
I am stating the issues I faced with pickling and MPI with the PSO module here for posterity. I was getting the following pickling error (identified after 2c16876).
I tried the following things that didn't work.
So, finally I resorted to using dill for pickling. Luckily, I added the dill functionality to schwimmbad sometime ago (adrn/schwimmbad@6bcc321). However, in my previous experience, I found dill to slow the computation down, expectedly because it's a pure-Python serialization, although I am not sure how much quantitatively. If it's a small hit, that should be fine. However, I am out of ideas to try for now (although, if emcee can work in MPI with default pickling, why can't the PSO module?) I am fine to use dill for now until someone (or me) finds the motivation to make things faster and to look for other ways to get MPI+PSO work with default pickling. I ran a short test run on hoffman2, and the 4-core run took about 1.7 times shorter than a 2-core run, so the MPI-distribution is working. |
Hi Anowar, thanks for looking into this in so much detail! I am happy to merge it for now and we can leave an issue open for the future. One way that might speed things up (but I don't know how to implement) is that the class instances do only need to be pickled/dill'ed once and only the functional arguments are then passed each time. All the likelihood computations can use the same instance of the likelihood class and the map function only needs to tell the arguments. |
Yes, please merge if you find no issues in the code. Your suggestion sounds effective, but I'm also not sure exactly how to implement it. An issue to track it would be nice. |
Add
class ParticleSwarmOptimizer()
and get rid of dependencies on cosmoHammer.