perf: batch array-valued distribution parameters into a single fill - #171
perf: batch array-valued distribution parameters into a single fill#171vchamarthi wants to merge 3 commits into
Conversation
| # [dev] (MM/DD/YYYY) | ||
|
|
||
| ### Added | ||
| * Added tests for the array-valued parameter paths of the location and scale distributions [gh-171](https://github.com/IntelPython/mkl_random/pull/171) |
There was a problem hiding this comment.
Tests relating note is not something the user might be interesting in. I'd propose to avoid adding such things in the changelog.
| ("laplace", lambda r, a, b, s: r.laplace(a, b, s), 2.0, 3.0), | ||
| ("gumbel", lambda r, a, b, s: r.gumbel(a, b, s), 2.0, 3.0), | ||
| ("logistic", lambda r, a, b, s: r.logistic(a, b, s), 2.0, 3.0), | ||
| ("lognormal", lambda r, a, b, s: r.lognormal(a, b, s), 0.5, 0.75), |
There was a problem hiding this comment.
We are testing only with default method=ICDF, and so no BoxMuller paths coverage.
| hi = np.linspace(pb, pb * 4.0, n) | ||
| out = draw(rnd.MKLRandomState(99), lo, hi, None) | ||
| first, last = out[: n // 4], out[-n // 4 :] | ||
| assert np.std(last) > np.std(first), ( |
There was a problem hiding this comment.
The per-element test is statistically weak.
If per-element scale were ignored, that comparison is a ~50/50 coin flip, so the test can pass even when the bug it targets is present. It also never sweeps a per-element loc. Strengthen to a magnitude check.
|
|
||
| ### Changed | ||
| * Sped up `normal`, `uniform`, `exponential`, `laplace`, `gumbel`, `logistic`, `rayleigh` and `lognormal` for array-valued parameters [gh-171](https://github.com/IntelPython/mkl_random/pull/171) | ||
| * The random streams for the array-valued-parameter paths of the distributions above have changed: with a fixed seed these now produce different (but equally valid) samples. Scalar-parameter paths are unaffected. [gh-171](https://github.com/IntelPython/mkl_random/pull/171) |
There was a problem hiding this comment.
| * The random streams for the array-valued-parameter paths of the distributions above have changed: with a fixed seed these now produce different (but equally valid) samples. Scalar-parameter paths are unaffected. [gh-171](https://github.com/IntelPython/mkl_random/pull/171) | |
| * Changed the random streams for the array-valued-parameter paths of the distributions above: with a fixed seed these now produce different (but equally valid) samples. Scalar-parameter paths are unaffected. `lognormal`'s Box-Muller path now draws through MKL's Gaussian `BOXMULLER2` generator. [gh-171](https://github.com/IntelPython/mkl_random/pull/171) |
| Upper boundary of the output interval. All values generated will be | ||
| less than high. The default value is 1.0. |
There was a problem hiding this comment.
The behavior changed now on array path:
- Old path (
vec_cont2_array→irk_uniform_vec(low, high)): MKL generated the values directly on the interval viaVSL_RNG_METHOD_UNIFORM_STD_ACCURATEwith the actuallow/high. MKL's uniform is generated on[low, high), so results stayed strictly below high. - New path (
vec_uniform_array): fills a standardU[0, 1)and reconstructs withlow + u·(high − low)vianp.subtract+np.multiply+np.add. Each of those three steps rounds to nearest double.
The new path is aligned how NumPy implements that, but need to align documentation also:
| Upper boundary of the output interval. All values generated will be | |
| less than high. The high limit may be included in the returned array of | |
| floats due to floating-point rounding in the equation | |
| ``low + (high-low) * random_sample()``. high - low must be | |
| non-negative. The default value is 1.0. |
There was a problem hiding this comment.
Also needs to add a note to the CHANGELOG that array-valued bounds may include high due to floating-point rounding
| return arr_obj | ||
|
|
||
|
|
||
| cdef object _param_out_shape(object size, tuple param_shapes): |
There was a problem hiding this comment.
It is stricter than the legacy check. This matches NumPy's own semantics and is one-directional (no previously-invalid call now passes). It's arguably a fix, but it's an acceptance-behavior change worth a changelog line.
What
When a distribution parameter is an array rather than a scalar,
vec_cont1_array/vec_cont2_arrayinvoke the fill once per element:That is 100,000 VSL calls for a 100,000-element array.
For location and scale families the parameters can be applied after the draw
instead, so one call covers the whole request:
These are exact identities, not approximations.
Routed:
normal,uniform,exponential,laplace,gumbel,logistic,rayleigh,lognormal.Not routed:
gamma,beta,f,chisquare,vonmises,wald,triangular,pareto,weibull,power, and the discrete distributions.Their parameters change the shape, so no affine step recovers them.
gammaiskept in the benchmark below as a control.
Numbers
Xeon Gold 6338, single thread, N=100k, ns/element, min of 2 runs. Intel-channel
mkl/mkl-devel2026.1.0-intel_236, meson build. Baseline and patchedinstalled in separate conda envs.
Scalar-parameter paths are untouched and not measured here.
Streams change
Array-parameter streams change: the draw is now standardised and transformed
rather than parameterised up front. Scalar-parameter paths are byte-identical.
Noted in the changelog.
Testing
master, so they are invariant guards rather than fitted to this change.
assert_allclose).crit 0.00364: all pass.
sizehandling and error cases compared against stocknumpy: agree, including incompatible
sizeraisingValueError.6b7962f.Notes for reviewers
_param_out_shapereimplements the shape and compatibility logic rather thanreusing the multi-iterator, since the batched path has no iterator. It raises
ValueError("size is not compatible with inputs")where the old pathsurfaced numpy's broadcast error text for some inputs. Same exception type,
different message.
lognormalis built on the normal fill, not the lognormal one: its parameterssit inside the exponential, so an affine step cannot apply them to a
standardised lognormal.
test_uniform_array_bounds_return_ndarraycovers this path and stillpasses.