You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Gilles Sadowski <gi...@gmail.com> on 2019/02/13 17:35:51 UTC
Re: [commons-rng] 02/02: The commons-math distributions can use a
null random generator
Hello.
Le mer. 13 févr. 2019 à 17:20, <ah...@apache.org> a écrit :
>
> This is an automated email from the ASF dual-hosted git repository.
>
> aherbert pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/commons-rng.git
>
> commit 367f022a88dedf2b79778f18ad1ed3ec655fffe8
> Author: aherbert <ah...@apache.org>
> AuthorDate: Wed Feb 13 16:20:21 2019 +0000
>
> The commons-math distributions can use a null random generator
Not a good move, I 'd think: this constructor has disappeared from
the development version of CM.[1]
Regards,
Gilles
[1] http://commons.apache.org/proper/commons-math/apidocs/org/apache/commons/math4/distribution/NormalDistribution.html
> ---
> .../distribution/ContinuousSamplersList.java | 74 ++++++++++++----------
> .../distribution/DiscreteSamplersList.java | 39 +++++++-----
> 2 files changed, 61 insertions(+), 52 deletions(-)
>
> diff --git a/commons-rng-sampling/src/test/java/org/apache/commons/rng/sampling/distribution/ContinuousSamplersList.java b/commons-rng-sampling/src/test/java/org/apache/commons/rng/sampling/distribution/ContinuousSamplersList.java
> index 58a2328..1cf4313 100644
> --- a/commons-rng-sampling/src/test/java/org/apache/commons/rng/sampling/distribution/ContinuousSamplersList.java
> +++ b/commons-rng-sampling/src/test/java/org/apache/commons/rng/sampling/distribution/ContinuousSamplersList.java
> @@ -33,172 +33,176 @@ public class ContinuousSamplersList {
>
> static {
> try {
> + // The commons-math distributions are not used for sampling so use a null random generator
> + org.apache.commons.math3.random.RandomGenerator rng = null;
> +
> // List of distributions to test.
>
> // Gaussian ("inverse method").
> final double meanNormal = -123.45;
> final double sigmaNormal = 6.789;
> - add(LIST, new org.apache.commons.math3.distribution.NormalDistribution(meanNormal, sigmaNormal),
> + add(LIST, new org.apache.commons.math3.distribution.NormalDistribution(rng, meanNormal, sigmaNormal),
> RandomSource.create(RandomSource.KISS));
> // Gaussian (DEPRECATED "Box-Muller").
> - add(LIST, new org.apache.commons.math3.distribution.NormalDistribution(meanNormal, sigmaNormal),
> + add(LIST, new org.apache.commons.math3.distribution.NormalDistribution(rng, meanNormal, sigmaNormal),
> new BoxMullerGaussianSampler(RandomSource.create(RandomSource.MT), meanNormal, sigmaNormal));
> // Gaussian ("Box-Muller").
> - add(LIST, new org.apache.commons.math3.distribution.NormalDistribution(meanNormal, sigmaNormal),
> + add(LIST, new org.apache.commons.math3.distribution.NormalDistribution(rng, meanNormal, sigmaNormal),
> new GaussianSampler(new BoxMullerNormalizedGaussianSampler(RandomSource.create(RandomSource.MT)),
> meanNormal, sigmaNormal));
> // Gaussian ("Marsaglia").
> - add(LIST, new org.apache.commons.math3.distribution.NormalDistribution(meanNormal, sigmaNormal),
> + add(LIST, new org.apache.commons.math3.distribution.NormalDistribution(rng, meanNormal, sigmaNormal),
> new GaussianSampler(new MarsagliaNormalizedGaussianSampler(RandomSource.create(RandomSource.MT)),
> meanNormal, sigmaNormal));
> // Gaussian ("Ziggurat").
> - add(LIST, new org.apache.commons.math3.distribution.NormalDistribution(meanNormal, sigmaNormal),
> + add(LIST, new org.apache.commons.math3.distribution.NormalDistribution(rng, meanNormal, sigmaNormal),
> new GaussianSampler(new ZigguratNormalizedGaussianSampler(RandomSource.create(RandomSource.MT)),
> meanNormal, sigmaNormal));
>
> // Beta ("inverse method").
> final double alphaBeta = 4.3;
> final double betaBeta = 2.1;
> - add(LIST, new org.apache.commons.math3.distribution.BetaDistribution(alphaBeta, betaBeta),
> + add(LIST, new org.apache.commons.math3.distribution.BetaDistribution(rng, alphaBeta, betaBeta),
> RandomSource.create(RandomSource.ISAAC));
> // Beta ("Cheng").
> - add(LIST, new org.apache.commons.math3.distribution.BetaDistribution(alphaBeta, betaBeta),
> + add(LIST, new org.apache.commons.math3.distribution.BetaDistribution(rng, alphaBeta, betaBeta),
> new ChengBetaSampler(RandomSource.create(RandomSource.MWC_256), alphaBeta, betaBeta));
> - add(LIST, new org.apache.commons.math3.distribution.BetaDistribution(betaBeta, alphaBeta),
> + add(LIST, new org.apache.commons.math3.distribution.BetaDistribution(rng, betaBeta, alphaBeta),
> new ChengBetaSampler(RandomSource.create(RandomSource.WELL_19937_A), betaBeta, alphaBeta));
> // Beta ("Cheng", alternate algorithm).
> final double alphaBetaAlt = 0.5678;
> final double betaBetaAlt = 0.1234;
> - add(LIST, new org.apache.commons.math3.distribution.BetaDistribution(alphaBetaAlt, betaBetaAlt),
> + add(LIST, new org.apache.commons.math3.distribution.BetaDistribution(rng, alphaBetaAlt, betaBetaAlt),
> new ChengBetaSampler(RandomSource.create(RandomSource.WELL_512_A), alphaBetaAlt, betaBetaAlt));
> - add(LIST, new org.apache.commons.math3.distribution.BetaDistribution(betaBetaAlt, alphaBetaAlt),
> + add(LIST, new org.apache.commons.math3.distribution.BetaDistribution(rng, betaBetaAlt, alphaBetaAlt),
> new ChengBetaSampler(RandomSource.create(RandomSource.WELL_19937_C), betaBetaAlt, alphaBetaAlt));
>
> // Cauchy ("inverse method").
> final double medianCauchy = 0.123;
> final double scaleCauchy = 4.5;
> - add(LIST, new org.apache.commons.math3.distribution.CauchyDistribution(medianCauchy, scaleCauchy),
> + add(LIST, new org.apache.commons.math3.distribution.CauchyDistribution(rng, medianCauchy, scaleCauchy),
> RandomSource.create(RandomSource.WELL_19937_C));
>
> // Chi-square ("inverse method").
> final int dofChi2 = 12;
> - add(LIST, new org.apache.commons.math3.distribution.ChiSquaredDistribution(dofChi2),
> + add(LIST, new org.apache.commons.math3.distribution.ChiSquaredDistribution(rng, dofChi2),
> RandomSource.create(RandomSource.WELL_19937_A));
>
> // Exponential ("inverse method").
> final double meanExp = 3.45;
> - add(LIST, new org.apache.commons.math3.distribution.ExponentialDistribution(meanExp),
> + add(LIST, new org.apache.commons.math3.distribution.ExponentialDistribution(rng, meanExp),
> RandomSource.create(RandomSource.WELL_44497_A));
> // Exponential.
> - add(LIST, new org.apache.commons.math3.distribution.ExponentialDistribution(meanExp),
> + add(LIST, new org.apache.commons.math3.distribution.ExponentialDistribution(rng, meanExp),
> new AhrensDieterExponentialSampler(RandomSource.create(RandomSource.MT), meanExp));
>
> // F ("inverse method").
> final int numDofF = 4;
> final int denomDofF = 7;
> - add(LIST, new org.apache.commons.math3.distribution.FDistribution(numDofF, denomDofF),
> + add(LIST, new org.apache.commons.math3.distribution.FDistribution(rng, numDofF, denomDofF),
> RandomSource.create(RandomSource.MT_64));
>
> // Gamma ("inverse method").
> final double thetaGammaSmallerThanOne = 0.1234;
> final double thetaGammaLargerThanOne = 2.345;
> final double alphaGamma = 3.456;
> - add(LIST, new org.apache.commons.math3.distribution.GammaDistribution(thetaGammaLargerThanOne, alphaGamma),
> + add(LIST, new org.apache.commons.math3.distribution.GammaDistribution(rng, thetaGammaLargerThanOne, alphaGamma),
> RandomSource.create(RandomSource.SPLIT_MIX_64));
> // Gamma (theta < 1).
> - add(LIST, new org.apache.commons.math3.distribution.GammaDistribution(thetaGammaSmallerThanOne, alphaGamma),
> + add(LIST, new org.apache.commons.math3.distribution.GammaDistribution(rng, thetaGammaSmallerThanOne, alphaGamma),
> new AhrensDieterMarsagliaTsangGammaSampler(RandomSource.create(RandomSource.XOR_SHIFT_1024_S),
> alphaGamma, thetaGammaSmallerThanOne));
> // Gamma (theta > 1).
> - add(LIST, new org.apache.commons.math3.distribution.GammaDistribution(thetaGammaLargerThanOne, alphaGamma),
> + add(LIST, new org.apache.commons.math3.distribution.GammaDistribution(rng, thetaGammaLargerThanOne, alphaGamma),
> new AhrensDieterMarsagliaTsangGammaSampler(RandomSource.create(RandomSource.WELL_44497_B),
> alphaGamma, thetaGammaLargerThanOne));
>
> // Gumbel ("inverse method").
> final double muGumbel = -4.56;
> final double betaGumbel = 0.123;
> - add(LIST, new org.apache.commons.math3.distribution.GumbelDistribution(muGumbel, betaGumbel),
> + add(LIST, new org.apache.commons.math3.distribution.GumbelDistribution(rng, muGumbel, betaGumbel),
> RandomSource.create(RandomSource.WELL_1024_A));
>
> // Laplace ("inverse method").
> final double muLaplace = 12.3;
> final double betaLaplace = 5.6;
> - add(LIST, new org.apache.commons.math3.distribution.LaplaceDistribution(muLaplace, betaLaplace),
> + add(LIST, new org.apache.commons.math3.distribution.LaplaceDistribution(rng, muLaplace, betaLaplace),
> RandomSource.create(RandomSource.MWC_256));
>
> // Levy ("inverse method").
> final double muLevy = -1.098;
> final double cLevy = 0.76;
> - add(LIST, new org.apache.commons.math3.distribution.LevyDistribution(muLevy, cLevy),
> + add(LIST, new org.apache.commons.math3.distribution.LevyDistribution(rng, muLevy, cLevy),
> RandomSource.create(RandomSource.TWO_CMRES));
>
> // Log normal ("inverse method").
> final double scaleLogNormal = 2.345;
> final double shapeLogNormal = 0.1234;
> - add(LIST, new org.apache.commons.math3.distribution.LogNormalDistribution(scaleLogNormal, shapeLogNormal),
> + add(LIST, new org.apache.commons.math3.distribution.LogNormalDistribution(rng, scaleLogNormal, shapeLogNormal),
> RandomSource.create(RandomSource.KISS));
> // Log-normal (DEPRECATED "Box-Muller").
> - add(LIST, new org.apache.commons.math3.distribution.LogNormalDistribution(scaleLogNormal, shapeLogNormal),
> + add(LIST, new org.apache.commons.math3.distribution.LogNormalDistribution(rng, scaleLogNormal, shapeLogNormal),
> new BoxMullerLogNormalSampler(RandomSource.create(RandomSource.XOR_SHIFT_1024_S), scaleLogNormal, shapeLogNormal));
> // Log-normal ("Box-Muller").
> - add(LIST, new org.apache.commons.math3.distribution.LogNormalDistribution(scaleLogNormal, shapeLogNormal),
> + add(LIST, new org.apache.commons.math3.distribution.LogNormalDistribution(rng, scaleLogNormal, shapeLogNormal),
> new LogNormalSampler(new BoxMullerNormalizedGaussianSampler(RandomSource.create(RandomSource.XOR_SHIFT_1024_S)),
> scaleLogNormal, shapeLogNormal));
> // Log-normal ("Marsaglia").
> - add(LIST, new org.apache.commons.math3.distribution.LogNormalDistribution(scaleLogNormal, shapeLogNormal),
> + add(LIST, new org.apache.commons.math3.distribution.LogNormalDistribution(rng, scaleLogNormal, shapeLogNormal),
> new LogNormalSampler(new MarsagliaNormalizedGaussianSampler(RandomSource.create(RandomSource.MT_64)),
> scaleLogNormal, shapeLogNormal));
> // Log-normal ("Ziggurat").
> - add(LIST, new org.apache.commons.math3.distribution.LogNormalDistribution(scaleLogNormal, shapeLogNormal),
> + add(LIST, new org.apache.commons.math3.distribution.LogNormalDistribution(rng, scaleLogNormal, shapeLogNormal),
> new LogNormalSampler(new ZigguratNormalizedGaussianSampler(RandomSource.create(RandomSource.MWC_256)),
> scaleLogNormal, shapeLogNormal));
>
> // Logistic ("inverse method").
> final double muLogistic = -123.456;
> final double sLogistic = 7.89;
> - add(LIST, new org.apache.commons.math3.distribution.LogisticDistribution(muLogistic, sLogistic),
> + add(LIST, new org.apache.commons.math3.distribution.LogisticDistribution(rng, muLogistic, sLogistic),
> RandomSource.create(RandomSource.TWO_CMRES_SELECT, null, 2, 6));
>
> // Nakagami ("inverse method").
> final double muNakagami = 78.9;
> final double omegaNakagami = 23.4;
> - add(LIST, new org.apache.commons.math3.distribution.NakagamiDistribution(muNakagami, omegaNakagami),
> + final double inverseAbsoluteAccuracyNakagami = org.apache.commons.math3.distribution.NakagamiDistribution.DEFAULT_INVERSE_ABSOLUTE_ACCURACY;
> + add(LIST, new org.apache.commons.math3.distribution.NakagamiDistribution(rng, muNakagami, omegaNakagami, inverseAbsoluteAccuracyNakagami),
> RandomSource.create(RandomSource.TWO_CMRES_SELECT, null, 5, 3));
>
> // Pareto ("inverse method").
> final double scalePareto = 23.45;
> final double shapePareto = 0.1234;
> - add(LIST, new org.apache.commons.math3.distribution.ParetoDistribution(scalePareto, shapePareto),
> + add(LIST, new org.apache.commons.math3.distribution.ParetoDistribution(rng, scalePareto, shapePareto),
> RandomSource.create(RandomSource.TWO_CMRES_SELECT, null, 9, 11));
> // Pareto.
> - add(LIST, new org.apache.commons.math3.distribution.ParetoDistribution(scalePareto, shapePareto),
> + add(LIST, new org.apache.commons.math3.distribution.ParetoDistribution(rng, scalePareto, shapePareto),
> new InverseTransformParetoSampler(RandomSource.create(RandomSource.XOR_SHIFT_1024_S), scalePareto, shapePareto));
>
> // T ("inverse method").
> final double dofT = 0.76543;
> - add(LIST, new org.apache.commons.math3.distribution.TDistribution(dofT),
> + add(LIST, new org.apache.commons.math3.distribution.TDistribution(rng, dofT),
> RandomSource.create(RandomSource.ISAAC));
>
> // Triangular ("inverse method").
> final double aTriangle = -0.76543;
> final double cTriangle = -0.65432;
> final double bTriangle = -0.54321;
> - add(LIST, new org.apache.commons.math3.distribution.TriangularDistribution(aTriangle, cTriangle, bTriangle),
> + add(LIST, new org.apache.commons.math3.distribution.TriangularDistribution(rng, aTriangle, cTriangle, bTriangle),
> RandomSource.create(RandomSource.MT));
>
> // Uniform ("inverse method").
> final double loUniform = -1.098;
> final double hiUniform = 0.76;
> - add(LIST, new org.apache.commons.math3.distribution.UniformRealDistribution(loUniform, hiUniform),
> + add(LIST, new org.apache.commons.math3.distribution.UniformRealDistribution(rng, loUniform, hiUniform),
> RandomSource.create(RandomSource.TWO_CMRES));
> // Uniform.
> - add(LIST, new org.apache.commons.math3.distribution.UniformRealDistribution(loUniform, hiUniform),
> + add(LIST, new org.apache.commons.math3.distribution.UniformRealDistribution(rng, loUniform, hiUniform),
> new ContinuousUniformSampler(RandomSource.create(RandomSource.MT_64), loUniform, hiUniform));
>
> // Weibull ("inverse method").
> final double alphaWeibull = 678.9;
> final double betaWeibull = 98.76;
> - add(LIST, new org.apache.commons.math3.distribution.WeibullDistribution(alphaWeibull, betaWeibull),
> + add(LIST, new org.apache.commons.math3.distribution.WeibullDistribution(rng, alphaWeibull, betaWeibull),
> RandomSource.create(RandomSource.WELL_44497_B));
> } catch (Exception e) {
> System.err.println("Unexpected exception while creating the list of samplers: " + e);
> diff --git a/commons-rng-sampling/src/test/java/org/apache/commons/rng/sampling/distribution/DiscreteSamplersList.java b/commons-rng-sampling/src/test/java/org/apache/commons/rng/sampling/distribution/DiscreteSamplersList.java
> index 10da78d..21cd20b 100644
> --- a/commons-rng-sampling/src/test/java/org/apache/commons/rng/sampling/distribution/DiscreteSamplersList.java
> +++ b/commons-rng-sampling/src/test/java/org/apache/commons/rng/sampling/distribution/DiscreteSamplersList.java
> @@ -35,18 +35,21 @@ public class DiscreteSamplersList {
>
> static {
> try {
> + // The commons-math distributions are not used for sampling so use a null random generator
> + org.apache.commons.math3.random.RandomGenerator rng = null;
> +
> // List of distributions to test.
>
> // Binomial ("inverse method").
> final int trialsBinomial = 20;
> final double probSuccessBinomial = 0.67;
> - add(LIST, new org.apache.commons.math3.distribution.BinomialDistribution(trialsBinomial, probSuccessBinomial),
> + add(LIST, new org.apache.commons.math3.distribution.BinomialDistribution(rng, trialsBinomial, probSuccessBinomial),
> MathArrays.sequence(8, 9, 1),
> RandomSource.create(RandomSource.KISS));
>
> // Geometric ("inverse method").
> final double probSuccessGeometric = 0.21;
> - add(LIST, new org.apache.commons.math3.distribution.GeometricDistribution(probSuccessGeometric),
> + add(LIST, new org.apache.commons.math3.distribution.GeometricDistribution(rng, probSuccessGeometric),
> MathArrays.sequence(10, 0, 1),
> RandomSource.create(RandomSource.ISAAC));
>
> @@ -54,80 +57,82 @@ public class DiscreteSamplersList {
> final int popSizeHyper = 34;
> final int numSuccessesHyper = 11;
> final int sampleSizeHyper = 12;
> - add(LIST, new org.apache.commons.math3.distribution.HypergeometricDistribution(popSizeHyper, numSuccessesHyper, sampleSizeHyper),
> + add(LIST, new org.apache.commons.math3.distribution.HypergeometricDistribution(rng, popSizeHyper, numSuccessesHyper, sampleSizeHyper),
> MathArrays.sequence(10, 0, 1),
> RandomSource.create(RandomSource.MT));
>
> // Pascal ("inverse method").
> final int numSuccessesPascal = 6;
> final double probSuccessPascal = 0.2;
> - add(LIST, new org.apache.commons.math3.distribution.PascalDistribution(numSuccessesPascal, probSuccessPascal),
> + add(LIST, new org.apache.commons.math3.distribution.PascalDistribution(rng, numSuccessesPascal, probSuccessPascal),
> MathArrays.sequence(18, 1, 1),
> RandomSource.create(RandomSource.TWO_CMRES));
>
> // Uniform ("inverse method").
> final int loUniform = -3;
> final int hiUniform = 4;
> - add(LIST, new org.apache.commons.math3.distribution.UniformIntegerDistribution(loUniform, hiUniform),
> + add(LIST, new org.apache.commons.math3.distribution.UniformIntegerDistribution(rng, loUniform, hiUniform),
> MathArrays.sequence(8, -3, 1),
> RandomSource.create(RandomSource.SPLIT_MIX_64));
> // Uniform.
> - add(LIST, new org.apache.commons.math3.distribution.UniformIntegerDistribution(loUniform, hiUniform),
> + add(LIST, new org.apache.commons.math3.distribution.UniformIntegerDistribution(rng, loUniform, hiUniform),
> MathArrays.sequence(8, -3, 1),
> new DiscreteUniformSampler(RandomSource.create(RandomSource.MT_64), loUniform, hiUniform));
> // Uniform (large range).
> final int halfMax = Integer.MAX_VALUE / 2;
> final int hiLargeUniform = halfMax + 10;
> final int loLargeUniform = -hiLargeUniform;
> - add(LIST, new org.apache.commons.math3.distribution.UniformIntegerDistribution(loLargeUniform, hiLargeUniform),
> + add(LIST, new org.apache.commons.math3.distribution.UniformIntegerDistribution(rng, loLargeUniform, hiLargeUniform),
> MathArrays.sequence(20, -halfMax, halfMax / 10),
> new DiscreteUniformSampler(RandomSource.create(RandomSource.WELL_1024_A), loLargeUniform, hiLargeUniform));
>
> // Zipf ("inverse method").
> final int numElementsZipf = 5;
> final double exponentZipf = 2.345;
> - add(LIST, new org.apache.commons.math3.distribution.ZipfDistribution(numElementsZipf, exponentZipf),
> + add(LIST, new org.apache.commons.math3.distribution.ZipfDistribution(rng, numElementsZipf, exponentZipf),
> MathArrays.sequence(5, 1, 1),
> RandomSource.create(RandomSource.XOR_SHIFT_1024_S));
> // Zipf.
> - add(LIST, new org.apache.commons.math3.distribution.ZipfDistribution(numElementsZipf, exponentZipf),
> + add(LIST, new org.apache.commons.math3.distribution.ZipfDistribution(rng, numElementsZipf, exponentZipf),
> MathArrays.sequence(5, 1, 1),
> new RejectionInversionZipfSampler(RandomSource.create(RandomSource.WELL_19937_C), numElementsZipf, exponentZipf));
> // Zipf (exponent close to 1).
> final double exponentCloseToOneZipf = 1 - 1e-10;
> - add(LIST, new org.apache.commons.math3.distribution.ZipfDistribution(numElementsZipf, exponentCloseToOneZipf),
> + add(LIST, new org.apache.commons.math3.distribution.ZipfDistribution(rng, numElementsZipf, exponentCloseToOneZipf),
> MathArrays.sequence(5, 1, 1),
> new RejectionInversionZipfSampler(RandomSource.create(RandomSource.WELL_19937_C), numElementsZipf, exponentCloseToOneZipf));
>
> // Poisson ("inverse method").
> + final double epsilonPoisson = org.apache.commons.math3.distribution.PoissonDistribution.DEFAULT_EPSILON;
> + final int maxIterationsPoisson = org.apache.commons.math3.distribution.PoissonDistribution.DEFAULT_MAX_ITERATIONS;
> final double meanPoisson = 3.21;
> - add(LIST, new org.apache.commons.math3.distribution.PoissonDistribution(meanPoisson),
> + add(LIST, new org.apache.commons.math3.distribution.PoissonDistribution(rng, meanPoisson, epsilonPoisson, maxIterationsPoisson),
> MathArrays.sequence(10, 0, 1),
> RandomSource.create(RandomSource.MWC_256));
> // Poisson.
> - add(LIST, new org.apache.commons.math3.distribution.PoissonDistribution(meanPoisson),
> + add(LIST, new org.apache.commons.math3.distribution.PoissonDistribution(rng, meanPoisson, epsilonPoisson, maxIterationsPoisson),
> MathArrays.sequence(10, 0, 1),
> new PoissonSampler(RandomSource.create(RandomSource.KISS), meanPoisson));
> // Dedicated small mean poisson sampler
> - add(LIST, new org.apache.commons.math3.distribution.PoissonDistribution(meanPoisson),
> + add(LIST, new org.apache.commons.math3.distribution.PoissonDistribution(rng, meanPoisson, epsilonPoisson, maxIterationsPoisson),
> MathArrays.sequence(10, 0, 1),
> new SmallMeanPoissonSampler(RandomSource.create(RandomSource.KISS), meanPoisson));
> // Poisson (40 < mean < 80).
> final double largeMeanPoisson = 67.89;
> - add(LIST, new org.apache.commons.math3.distribution.PoissonDistribution(largeMeanPoisson),
> + add(LIST, new org.apache.commons.math3.distribution.PoissonDistribution(rng, largeMeanPoisson, epsilonPoisson, maxIterationsPoisson),
> MathArrays.sequence(50, (int) (largeMeanPoisson - 25), 1),
> new PoissonSampler(RandomSource.create(RandomSource.SPLIT_MIX_64), largeMeanPoisson));
> // Dedicated large mean poisson sampler
> - add(LIST, new org.apache.commons.math3.distribution.PoissonDistribution(largeMeanPoisson),
> + add(LIST, new org.apache.commons.math3.distribution.PoissonDistribution(rng, largeMeanPoisson, epsilonPoisson, maxIterationsPoisson),
> MathArrays.sequence(50, (int) (largeMeanPoisson - 25), 1),
> new LargeMeanPoissonSampler(RandomSource.create(RandomSource.SPLIT_MIX_64), largeMeanPoisson));
> // Poisson (mean >> 40).
> final double veryLargeMeanPoisson = 543.21;
> - add(LIST, new org.apache.commons.math3.distribution.PoissonDistribution(veryLargeMeanPoisson),
> + add(LIST, new org.apache.commons.math3.distribution.PoissonDistribution(rng, veryLargeMeanPoisson, epsilonPoisson, maxIterationsPoisson),
> MathArrays.sequence(100, (int) (veryLargeMeanPoisson - 50), 1),
> new PoissonSampler(RandomSource.create(RandomSource.SPLIT_MIX_64), veryLargeMeanPoisson));
> // Dedicated large mean poisson sampler
> - add(LIST, new org.apache.commons.math3.distribution.PoissonDistribution(veryLargeMeanPoisson),
> + add(LIST, new org.apache.commons.math3.distribution.PoissonDistribution(rng, veryLargeMeanPoisson, epsilonPoisson, maxIterationsPoisson),
> MathArrays.sequence(100, (int) (veryLargeMeanPoisson - 50), 1),
> new LargeMeanPoissonSampler(RandomSource.create(RandomSource.SPLIT_MIX_64), veryLargeMeanPoisson));
> } catch (Exception e) {
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [commons-rng] 02/02: The commons-math distributions can use a
null random generator
Posted by Alex Herbert <al...@gmail.com>.
On 14/02/2019 12:42, Gilles Sadowski wrote:
> Hello Herbert.
>
> Le jeu. 14 févr. 2019 à 12:58, Alex Herbert <al...@gmail.com> a écrit :
>> On 13/02/2019 22:28, Gilles Sadowski wrote:
>>> Hi.
>>>
>>> Le mer. 13 févr. 2019 à 21:21, Alex Herbert <al...@gmail.com> a écrit :
>>>>
>>>>> On 13 Feb 2019, at 17:52, Gilles Sadowski <gi...@gmail.com> wrote:
>>>>>
>>>>>> Le mer. 13 févr. 2019 à 18:47, Alex Herbert <al...@gmail.com> a écrit :
>>>>>>
>>>>>>> On 13/02/2019 17:35, Gilles Sadowski wrote:
>>>>>>> Hello.
>>>>>>>
>>>>>>>> Le mer. 13 févr. 2019 à 17:20, <ah...@apache.org> a écrit :
>>>>>>>> This is an automated email from the ASF dual-hosted git repository.
>>>>>>>>
>>>>>>>> aherbert pushed a commit to branch master
>>>>>>>> in repository https://gitbox.apache.org/repos/asf/commons-rng.git
>>>>>>>>
>>>>>>>> commit 367f022a88dedf2b79778f18ad1ed3ec655fffe8
>>>>>>>> Author: aherbert <ah...@apache.org>
>>>>>>>> AuthorDate: Wed Feb 13 16:20:21 2019 +0000
>>>>>>>>
>>>>>>>> The commons-math distributions can use a null random generator
>>>>>>> Not a good move, I 'd think: this constructor has disappeared from
>>>>>>> the development version of CM.[1]
>>>>>> The constructor is in the unit tests. These depend on commons-math3 in
>>>>>> the test scope.
>>>>>>
>>>>>> If the tests are upgraded to commons-math4 then this will just be a
>>>>>> compile error. The tests would have to be changed anyway to reference
>>>>>> the updated component package name for maths.
>>>>> I know; the point is that this constructor was a bad design
>>>>> decision (there are old and long discussions about it).
>>>>> So, why present wrong usage in [RNG] (that would a.o. lead
>>>>> to wondering why the constructor was deleted)?
>>>>>
>>>> I thought I was changing to the recommended usage.
>>>>
>>>> The javadoc for the distributions in CM acknowledge that a generator is created by default unless you supply one, and that it is used for sampling. Given the tests are not sampling then null is recommended.
>>>>
>>>> The Junit test code can be updated with a better comment to reflect the recommended usage. Currently it states: “The commons-math distributions are not used for sampling so use a null random generator”. I can change to the CM javadoc of: “In case no sampling is needed for the created distribution, it is advised to pass null as random generator via the appropriate constructors to avoid the additional initialisation overhead.”
>>>>
>>>> I can rename the RandomGenerator to ‘unusedRng’. Anyone who reads the code should not be mislead by this.
>>> I'm sorry for the perpetuation of the misunderstanding.
>>> The CM classes were wrong in initializing a RNG instance even though
>>> it might never be used. Hence the new API there[1] passes the RNG only
>>> when the user wants to sample.
>>> Passing null to the (deprecated) constructor is a workaround to avoid the
>>> unnecessary creation, as you note but it thus perpetuates bad usage; in
>>> the case of a unit test, documenting proper use is IMO more important than
>>> having a slight performance improvement.
>>> Note that if/when we release [Statisitics], the classes from there (where the
>>> API has been corrected) will be used in the test.
>>>
>>> HTH,
>>> Gilles
>>>
>>> [1] http://commons.apache.org/proper/commons-math/apidocs/src-html/org/apache/commons/math4/distribution/NormalDistribution.html#line.240
>> We seem to have some crossed wires here. I am advocating using the CM3
>> distributions in the recommended way. In that library these constructors
>> are not deprecated. So this is a change for 'proper use' of that
>> library. You are advocating using the constructor as it appears in the
>> improved, but unreleased, CM4 distributions. This is to promote a better
>> designed library that accept only parameters related to the distribution
>> and also avoid a reader wondering why the CM3 constructor was deleted in
>> CM4. I can see the merit in this. However as advocates of commons I
>> believe we should employ the recommended way for the library we are
>> using irrespective of what will happen in the future.
>>
>> Given the scenario that a user reads the test code to understand how the
>> test is checking the samplers do sample correctly from the distribution.
>> Can we change the test code to state that the reference distribution is
>> constructed using CM3. Here the distributions have a dual functionality
>> of computing the PMF/PDF and also performing sampling. The test does not
>> require the sampling functionality and so passes an 'unusedRng' to the
>> constructor. This should not lead any reader astray, promotes correct
>> usage of CM3 and part-way explains why CM4 does not have the constructor
>> (since the distributions now have a separate sampling functionality).
>>
>> In a flip scenario if the original code is restored then a reader would
>> have an example of how to use CM3 for PMF/PDF computations that has a
>> performance hit (that they may not know about). It would be up to them
>> to discover this and then how to avoid this performance hit by reading
>> the documentation. I think this is worse, hence the change.
>>
>> So the lesser of two evils: (1) show a user how to avoid poor
>> performance during usage of CM3 but prompt them to question the library
>> design; (2) show a user how to use a distribution with only the
>> parameters that control the distribution, but leave them with a
>> performance hit to figure out if they copy the code.
>>
>> Thank you for making me clarify my position. If you still disagree then
>> we can restore the old code and put a note that the distributions in
>> future should be from commons statistics/CM4 (or create a Jira ticket to
>> record the intention). I'm happy with either. But in a reversion I would
>> add a note that for clarity the CM3 implementation here is used with
>> only the parameters that control the distribution. Other parameters that
>> affect performance are not employed.
> I can understand what you say; it is a rehash of all the discussions
> we had back then. ;-)
> The (so-called "recommended") null argument was to be a workaround
> for users that could not bear the performance hit.
>
> There is no real need to revert the code as long as it won't entail
> questioning that the design fix, as implemented in [Statisitics], is
> fine, and that the test in [RNG] will use that in due time.
>
> What would be great is to review [Statistics] and make a release
> of it[1], so that we can drop the dependency towards CM v3.6.1 and
> stop the indirect advocacy for an outdated version just because
> CM 4.0 (with tens of bugfixes and enhancements) could never be
> released.
>
> [1] Even if the version number would "0.x" because its scope is much
> larger than its current contents.
Sorry this went off list in my last reply.
So the current code using the null RandomGenerator stays. I can update
the test with a better explanation quoting the current Javadoc for the
distribution, e.g.:
// This test uses reference distributions from
commons-math3 to compute the expected
// PMF. These distributions have a dual functionality to
compute the PMF and perform
// sampling. In case no sampling is needed for the created
distribution, it is advised
// to pass null as the random generator via the appropriate
constructors to avoid the
// additional initialisation overhead.
org.apache.commons.math3.random.RandomGenerator unusedRng =
null;
I will put it on my todo list to have a look at [Statistics].
Alex
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [commons-rng] 02/02: The commons-math distributions can use a
null random generator
Posted by Gilles Sadowski <gi...@gmail.com>.
Hi.
Le mer. 13 févr. 2019 à 21:21, Alex Herbert <al...@gmail.com> a écrit :
>
>
>
> > On 13 Feb 2019, at 17:52, Gilles Sadowski <gi...@gmail.com> wrote:
> >
> >> Le mer. 13 févr. 2019 à 18:47, Alex Herbert <al...@gmail.com> a écrit :
> >>
> >>> On 13/02/2019 17:35, Gilles Sadowski wrote:
> >>> Hello.
> >>>
> >>>> Le mer. 13 févr. 2019 à 17:20, <ah...@apache.org> a écrit :
> >>>> This is an automated email from the ASF dual-hosted git repository.
> >>>>
> >>>> aherbert pushed a commit to branch master
> >>>> in repository https://gitbox.apache.org/repos/asf/commons-rng.git
> >>>>
> >>>> commit 367f022a88dedf2b79778f18ad1ed3ec655fffe8
> >>>> Author: aherbert <ah...@apache.org>
> >>>> AuthorDate: Wed Feb 13 16:20:21 2019 +0000
> >>>>
> >>>> The commons-math distributions can use a null random generator
> >>> Not a good move, I 'd think: this constructor has disappeared from
> >>> the development version of CM.[1]
> >>
> >> The constructor is in the unit tests. These depend on commons-math3 in
> >> the test scope.
> >>
> >> If the tests are upgraded to commons-math4 then this will just be a
> >> compile error. The tests would have to be changed anyway to reference
> >> the updated component package name for maths.
> >
> > I know; the point is that this constructor was a bad design
> > decision (there are old and long discussions about it).
> > So, why present wrong usage in [RNG] (that would a.o. lead
> > to wondering why the constructor was deleted)?
> >
>
> I thought I was changing to the recommended usage.
>
> The javadoc for the distributions in CM acknowledge that a generator is created by default unless you supply one, and that it is used for sampling. Given the tests are not sampling then null is recommended.
>
> The Junit test code can be updated with a better comment to reflect the recommended usage. Currently it states: “The commons-math distributions are not used for sampling so use a null random generator”. I can change to the CM javadoc of: “In case no sampling is needed for the created distribution, it is advised to pass null as random generator via the appropriate constructors to avoid the additional initialisation overhead.”
>
> I can rename the RandomGenerator to ‘unusedRng’. Anyone who reads the code should not be mislead by this.
I'm sorry for the perpetuation of the misunderstanding.
The CM classes were wrong in initializing a RNG instance even though
it might never be used. Hence the new API there[1] passes the RNG only
when the user wants to sample.
Passing null to the (deprecated) constructor is a workaround to avoid the
unnecessary creation, as you note but it thus perpetuates bad usage; in
the case of a unit test, documenting proper use is IMO more important than
having a slight performance improvement.
Note that if/when we release [Statisitics], the classes from there (where the
API has been corrected) will be used in the test.
HTH,
Gilles
[1] http://commons.apache.org/proper/commons-math/apidocs/src-html/org/apache/commons/math4/distribution/NormalDistribution.html#line.240
>
> > Regards,
> > Gilles
> >
> >>
> >> Alex
> >>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [commons-rng] 02/02: The commons-math distributions can use a null random generator
Posted by Alex Herbert <al...@gmail.com>.
> On 13 Feb 2019, at 17:52, Gilles Sadowski <gi...@gmail.com> wrote:
>
>> Le mer. 13 févr. 2019 à 18:47, Alex Herbert <al...@gmail.com> a écrit :
>>
>>> On 13/02/2019 17:35, Gilles Sadowski wrote:
>>> Hello.
>>>
>>>> Le mer. 13 févr. 2019 à 17:20, <ah...@apache.org> a écrit :
>>>> This is an automated email from the ASF dual-hosted git repository.
>>>>
>>>> aherbert pushed a commit to branch master
>>>> in repository https://gitbox.apache.org/repos/asf/commons-rng.git
>>>>
>>>> commit 367f022a88dedf2b79778f18ad1ed3ec655fffe8
>>>> Author: aherbert <ah...@apache.org>
>>>> AuthorDate: Wed Feb 13 16:20:21 2019 +0000
>>>>
>>>> The commons-math distributions can use a null random generator
>>> Not a good move, I 'd think: this constructor has disappeared from
>>> the development version of CM.[1]
>>
>> The constructor is in the unit tests. These depend on commons-math3 in
>> the test scope.
>>
>> If the tests are upgraded to commons-math4 then this will just be a
>> compile error. The tests would have to be changed anyway to reference
>> the updated component package name for maths.
>
> I know; the point is that this constructor was a bad design
> decision (there are old and long discussions about it).
> So, why present wrong usage in [RNG] (that would a.o. lead
> to wondering why the constructor was deleted)?
>
I thought I was changing to the recommended usage.
The javadoc for the distributions in CM acknowledge that a generator is created by default unless you supply one, and that it is used for sampling. Given the tests are not sampling then null is recommended.
The Junit test code can be updated with a better comment to reflect the recommended usage. Currently it states: “The commons-math distributions are not used for sampling so use a null random generator”. I can change to the CM javadoc of: “In case no sampling is needed for the created distribution, it is advised to pass null as random generator via the appropriate constructors to avoid the additional initialisation overhead.”
I can rename the RandomGenerator to ‘unusedRng’. Anyone who reads the code should not be mislead by this.
> Regards,
> Gilles
>
>>
>> Alex
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
Re: [commons-rng] 02/02: The commons-math distributions can use a
null random generator
Posted by Gilles Sadowski <gi...@gmail.com>.
Le mer. 13 févr. 2019 à 18:47, Alex Herbert <al...@gmail.com> a écrit :
>
> On 13/02/2019 17:35, Gilles Sadowski wrote:
> > Hello.
> >
> > Le mer. 13 févr. 2019 à 17:20, <ah...@apache.org> a écrit :
> >> This is an automated email from the ASF dual-hosted git repository.
> >>
> >> aherbert pushed a commit to branch master
> >> in repository https://gitbox.apache.org/repos/asf/commons-rng.git
> >>
> >> commit 367f022a88dedf2b79778f18ad1ed3ec655fffe8
> >> Author: aherbert <ah...@apache.org>
> >> AuthorDate: Wed Feb 13 16:20:21 2019 +0000
> >>
> >> The commons-math distributions can use a null random generator
> > Not a good move, I 'd think: this constructor has disappeared from
> > the development version of CM.[1]
>
> The constructor is in the unit tests. These depend on commons-math3 in
> the test scope.
>
> If the tests are upgraded to commons-math4 then this will just be a
> compile error. The tests would have to be changed anyway to reference
> the updated component package name for maths.
I know; the point is that this constructor was a bad design
decision (there are old and long discussions about it).
So, why present wrong usage in [RNG] (that would a.o. lead
to wondering why the constructor was deleted)?
Regards,
Gilles
>
> Alex
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [commons-rng] 02/02: The commons-math distributions can use a
null random generator
Posted by Alex Herbert <al...@gmail.com>.
On 13/02/2019 17:35, Gilles Sadowski wrote:
> Hello.
>
> Le mer. 13 févr. 2019 à 17:20, <ah...@apache.org> a écrit :
>> This is an automated email from the ASF dual-hosted git repository.
>>
>> aherbert pushed a commit to branch master
>> in repository https://gitbox.apache.org/repos/asf/commons-rng.git
>>
>> commit 367f022a88dedf2b79778f18ad1ed3ec655fffe8
>> Author: aherbert <ah...@apache.org>
>> AuthorDate: Wed Feb 13 16:20:21 2019 +0000
>>
>> The commons-math distributions can use a null random generator
> Not a good move, I 'd think: this constructor has disappeared from
> the development version of CM.[1]
The constructor is in the unit tests. These depend on commons-math3 in
the test scope.
If the tests are upgraded to commons-math4 then this will just be a
compile error. The tests would have to be changed anyway to reference
the updated component package name for maths.
Alex
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org