probvec: Use guvectorize with target='parallel'

[oyamad]

This is to modify probvec to use guvectorize with target='parallel'.
(probvec is mainly used to generate samples of MarkovChain or DiscreteDP.)

• Use of guvectorize simplifies the logic slightly; we don't need to explicitly write the outer loop.
• This version puts sort() in a jitted loop with nopython mode, the bottleneck of the current probvec.
• Sorting by Numba may be slower than Numpy, but with target='parallel' this version is faster than the current version on master for cases with large size, whereas it seems slower with small size (probably due to overhead).

Timing on a machine with 4 cores:

master:

In [1]: import quantecon as qe

In [2]: seed = 1234

In [3]: m, k = 5000, 5000

In [4]: %timeit qe.random.probvec(m, k, random_state=seed)
1 loop, best of 3: 1.63 s per loop

In [5]: beta = 0.95

In [6]: num_states, num_actions = 500, 100

In [7]: %timeit qe.markov.random_discrete_dp(num_states, num_actions, beta, random_state=seed)
1 loop, best of 3: 1.45 s per loop

This version:

In [1]: import quantecon as qe

In [2]: seed = 1234

In [3]: m, k = 5000, 5000

In [4]: %timeit qe.random.probvec(m, k, random_state=seed)
1 loop, best of 3: 681 ms per loop

In [5]: beta = 0.95

In [6]: num_states, num_actions = 500, 100

In [7]: %timeit qe.markov.random_discrete_dp(num_states, num_actions, beta, random_state=seed)
1 loop, best of 3: 747 ms per loop

This is the first case in quantecon where guvectorize is used; we might discuss its pros and cons here.

[coveralls]

Coverage decreased (-0.003%) to 86.693% when pulling 102fc7a on probvec-guvectorize into a8ff846 on master.

[sglyon]

I'm not opposed to using guvectorize at any point in the library, but I don't love this particular change. The reason is that if I was using these tools within a larger algorithm, I would almost certainly want to parallelize a different section of my code. Having this routine always run in parallel might make that difficult.

[oyamad]

@spencerlyon2 We shouldn't accept this PR if it can interfere anything more important; probvec is just used mainly (or only) for benchmark testing.

But can you elaborate on your point? Do you have in mind any case where you may use this particular routine within a larger algorithm?

[mmcky]

Hi @oyamad and @spencerlyon2. Where are we up to with this PR? Should we add the guvectorize / jit as an option to a top level function (with default to jit?)

[sglyon]

I thought about it more. I vote that we accept this.

It would be great it numba allowed us to use guvectorize and then select the execution target at runtime instead of compile time, but I don't think that is possible.

[oyamad]

We could possibly do:

def probvec(m, k, random_state=None, parallel=True):
    ...
    if parallel:
        _probvec_parallel(r, x)
    else:
        _probvec_cpu(r, x)


def _probvec(r, out):
    ...

_probvec_parallel =
    guvectorize(['(f8[:], f8[:])'], '(n), (k)', nopython=True, target='parallel')(_probvec)

_probvec_cpu =
    guvectorize(['(f8[:], f8[:])'], '(n), (k)', nopython=True, target='cpu')(_probvec)

[mmcky]

@spencerlyon2 Are you still concerned with:

Having this routine always run in parallel might make that difficult.

We can implement options through a wrapper (as per @oyamad solution) or make parallel the default.

[sglyon]

I think the proposed solution sounds great. Let's go with that!
On Jun 27, 2016 5:09 AM, "mmcky" [email protected] wrote:

@spencerlyon2 https://github.com/spencerlyon2 Are you still concerned
with:

Having this routine always run in parallel might make that difficult.

We can implement options through a wrapper (as per @oyamad
https://github.com/oyamad solution) or make parallel the default.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#253 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AA9J3Ic1HS7bFzQg_-JSLYoWuKiFB9fXks5qP-d_gaJpZM4IJLXu
.

[coveralls]

Coverage decreased (-0.1%) to 86.591% when pulling 0fcdf56 on probvec-guvectorize into a8ff846 on master.

[mmcky]

@oyamad please let me know if you are happy with this update then we can merge this into the master branch. Thanks.

[coveralls]

Coverage decreased (-0.05%) to 86.645% when pulling d1d87e6 on probvec-guvectorize into a8ff846 on master.

[oyamad]

@mmcky Thanks, please do merge. (I added some docstring and tests.)

[mmcky]

Thanks @oyamad! Merging now.