Le ven. 23 juil. 2021 à 23:23, Alex Herbert <[email protected]> a écrit :
>
> When updating the documentation for new samplers I noticed that the shape
> samplers accepted the RNG as the last argument to the factory constructor
> methods:
>
> double[] a = {1, 2}
> double[] b = {3, 4}
> UniformRandomProvider rng = RandonSouce.KISS.create()
> BoxSampler sampler = BoxSampler.of(a, b, rng);
>
> All the distribution samplers take the RNG as the first argument. The other
> utility samplers for collections and combinations/permutations also take
> the RNG as the first argument.
>
> The shape samplers were based on the UnitSphereSampler which takes the RNG
> as the last argument. It appears to be an odd-one-out.
>
> I have updated the shape samplers to take the RNG as the first argument:
>
> BoxSampler sampler = BoxSampler.of(rng, a, b);
>
> The constructor for the UnitSphereSampler cannot be changed due to BC. But
> the factory constructor is currently unreleased. This could be changed to
> accept the RNG as the first argument. It would make upgrading code more
> difficult to achieve with a simple regex:
>
> int dimension = 3;
> UnitSphereSampler s1 = new UnitSphereSampler(dimension, rng);
> UnitSphereSampler s2 = UnitSphereSampler.of(rng, dimension);
>
> IMO we should set the RNG as the first argument to the factory constructor
> for consistency across the entire package. I would also mark the public
> constructor as deprecated. The public constructor creates a wrapper class
> around a delegate which implements sampling. Use of the factory constructor
> directly creates the optimal sampler. So use of the constructor should be
> avoided. Marking it as deprecated would encourage users to change to the
> optimal factory constructor.
>
> Any opinions on this?

Agreed.

Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to