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

Provides optional parallel running of repetitions #116

Merged
merged 26 commits into from
Mar 20, 2015
Merged

Provides optional parallel running of repetitions #116

merged 26 commits into from
Mar 20, 2015

Conversation

meatballs
Copy link
Member

  • Adds run_serial_repetitions and run_parallel_repetitions methods to Tournament
  • Adds new argument to run_tournament (-p or --processes) to define number of parallel processes to run. The default is serial processing. Actual number used is limited to the cpu_count.
  • Refactors playing of round robin and updating of result set out of Tournment.play and into separate methods
  • The result set update from above now lives within ResultSet.finalise (renamed from ResultSet.init_output)
  • Passes logger from TournamentManager to Tournament to log number of processes in use
  • Updates test suite to match

And possibly other things I've forgotten.

And possibly not some things I've overlooked!

@meatballs
Copy link
Member Author

On my laptop, running all tournaments serially takes approx. 90s. Running with -p 0 takes this down to approx. 75 seconds

@meatballs
Copy link
Member Author

I'm finished with this one. It's ready to roll.

@drvinceknight
Copy link
Member

@JasYoung314 can you also throw your eyes at this one.

@meatballs
Copy link
Member Author

According to various online sources (e.g. http://stackoverflow.com/questions/10037319/how-do-i-tell-if-pythons-multiprocessing-module-is-using-all-of-my-cores-for-ca) python's multiprocessing.cpu_count will return double the actual number of cores for Intel CPUs with hyper threading.

On my Macbook Air, 'About my Mac' shows it has an Intel I7 with 2 cores. cpu_count returns 4.

Running run_tournament with -p 0 launches 4 processes and completes in approx. 75 seconds (as quoted previously). Running with -p 2 reduces this further to 70 seconds.

@JasYoung315
Copy link
Contributor

I've just taken a look over it: Looks great!

It's very similar to a way I had previously implemented parallel processing.

Looking through the code, should line 74 in the tournament.py file be

if self.processes < 2 or self.processes > multiprocessing.cpu_count():

I've left a comment in the commit itself.

work_queue = multiprocessing.Queue()
done_queue = multiprocessing.Queue()

if self.processes < 2 or self.processes > multiprocessing.cpu_count:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be

if self.processes < 2 or self.processes > multiprocessing.cpu_count():

@meatballs
Copy link
Member Author

Looking through the code, should line 74 in the tournament.py file be

if self.processes < 2 or self.processes > multiprocessing.cpu_count():

Yep -that's a typo! Presumably I got away with it simply because it was treated as a property accessor.

@JasYoung315
Copy link
Contributor

It's actually because in python, certain types have a notion of ordering. It's arbitrary but consistent, so you could compare the function cpu_count to the integer self.processes and have the following results

>>> multiprocessing.cpu_count()
4
>>> 5 < multiprocessing.cpu_count
True
>>> 5 > multiprocessing.cpu_count
False

because the type integer is always considered to be less than the type function. More information (and a much more informed explanation can be found here)

Pull request seems awesome!

@meatballs
Copy link
Member Author

Thank you @JasYoung314 for both the explanation and the compliment!!

(Is now a good time to own up to the fact that I'm only really doing this because I've not used Python before and fancied an excuse to learn)?

@JasYoung315
Copy link
Contributor

(I think the time to own up was before you started messing with multiprocessing! Now you're one of us I'm afraid)

@drvinceknight
Copy link
Member

Just going through it now: this is very nice to see!

screenshot 2015-03-20 10 12 55

@drvinceknight drvinceknight merged commit c2f8e4a into Axelrod-Python:master Mar 20, 2015
@drvinceknight
Copy link
Member

I added something to the README also. Thanks for this @meatballs : huge contribution.

@drvinceknight
Copy link
Member

Thanks a lot @JasYoung314 for reviewing this.

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.

None yet

3 participants