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

Reduced the number of processes used by default when testing. #2888

Merged
merged 1 commit into from
Oct 31, 2017

Conversation

pelson
Copy link
Member

@pelson pelson commented Oct 28, 2017

Given we are far more parallelised now in iris (thanks to our new-found dask abilities) we don't need to use as many processes in parallel when testing. I didn't do any particularly sophisticated analysis of the "sweet spot", but leave that as an exercise for the interested reader...

In summary: we don't need to flood the machine with processes, and anything is better than what we currently do...

On Travis (where the numbers are extremely erratic), using 8 processes appears to take a similar amount of time to using 3. On my laptop (4 core), using 1 process takes about the same amount of time as 3.

@pelson pelson force-pushed the fewer_parallel_test branch from cd963cd to 9253f2e Compare October 30, 2017 08:39
@pelson pelson added this to the v2.0.0 milestone Oct 30, 2017
@pelson
Copy link
Member Author

pelson commented Oct 30, 2017

I don't think it is a fluke that there are not TimeoutExceptions here... 🎉

self.num_processors = multiprocessing.cpu_count() - 1
# Choose a magic number that works reasonably well for the default
# number of processes.
self.num_processors = (multiprocessing.cpu_count() + 1) // 4 + 1
Copy link
Member

Choose a reason for hiding this comment

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

@pelson What's the logic behind this lovely piece of maths?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a magic number... there is no solid/defensible logic other than:

  • It should be at least one
  • It shouldn't fill all CPUs if there are lots of them

😄

@DPeterK DPeterK merged commit 74b210c into SciTools:master Oct 31, 2017
@pelson pelson mentioned this pull request Oct 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants