[
https://issues.apache.org/jira/browse/RNG-96?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16826007#comment-16826007
]
Alex D Herbert commented on RNG-96:
-----------------------------------
Yes it is a naming mismatch similar to MATH-1373.
The MathWorld link labels the parameters alpha and theta. Their definitions of
skewness and kurtosis use the alpha parameter.
The Wikipedia link labels the parameters k or alpha as shape and theta as
scale. With these definitions the skewness and kurtosis also use the k or alpha
parameter.
So MathWorld and Wikipedia match their definitions.
The variance uses both parameters so can be used as a test:
{noformat}
variance = alpha * theta^2
{noformat}
A simple test shows that the distribution is wrong according to this:
{code:java}
@Test
public void testVariance() {
checkVariance(1, 2);
}
private void checkVariance(double alpha, double theta) {
final RestorableUniformRandomProvider rng =
RandomSource.create(RandomSource.SPLIT_MIX_64);
final AhrensDieterMarsagliaTsangGammaSampler sampler =
new AhrensDieterMarsagliaTsangGammaSampler(rng, alpha, theta);
final SummaryStatistics stats = new SummaryStatistics();
for (int i = 0; i < 10000; i++) {
stats.addValue(sampler.sample());
}
// From https://en.wikipedia.org/wiki/Gamma_distribution
final double expected = alpha * theta * theta;
Assert.assertEquals(expected, stats.getVariance(), expected * 0.1);
}
{code}
This fails but when alpha and theta are reversed for the constructor it passes.
So should I do a PR that just updates the names and Javadoc and maintain the
usage of the input parameters?
Or a more breaking change to maintain the parameter order and update the class
internals to correctly use the input parameters as named? This would break any
code of a user who is currently running the sampler with a known workaround of
swapping the parameters and did not think to raise a bug. But it will fix the
code of any user who read the javadoc, passed the parameters as stated and is
getting incorrect samples.
> AhrensDieterMarsagliaTsangGammaSampler incorrectly names parameters
> -------------------------------------------------------------------
>
> Key: RNG-96
> URL: https://issues.apache.org/jira/browse/RNG-96
> Project: Commons RNG
> Issue Type: Bug
> Components: sampling
> Affects Versions: 1.3
> Reporter: Alex D Herbert
> Assignee: Alex D Herbert
> Priority: Minor
> Fix For: 1.3
>
>
> The AhrensDieterMarsagliaTsangGammaSampler has parameters alpha and theta.
> Using the naming conventions on [Wikipedia Gamma
> distribution|https://en.wikipedia.org/wiki/Gamma_distribution] the alpha
> parameter is also known as the shape and the theta parameter is the scale:
> {noformat}
> // Wikipedia
> alpha = shape
> theta = scale
> {noformat}
> However if the sampler is run with the same parameters as the Wikipedia
> article histograms of the output sample distribution does not match. They
> need to be swapped indicating a naming mismatch.
> Studying the same algorithm in o.a.c.math3.distribution.GammaDistribution it
> appears that the theta parameter is being used by Commons RNG as the shape
> and the alpha parameter is being used as the scale. So the names are
> incorrect and have been swapped.
> {noformat}
> // Commons RNG
> alpha = scale (wrong)
> theta = shape (wrong)
> {noformat}
> The unit tests for this sampler does this:
> {code:java}
> // Gamma (theta < 1).
> add(LIST, new
> org.apache.commons.math3.distribution.GammaDistribution(unusedRng,
> 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(unusedRng,
> thetaGammaLargerThanOne, alphaGamma),
> new AhrensDieterMarsagliaTsangGammaSampler(
> RandomSource.create(RandomSource.WELL_44497_B),
>
> alphaGamma, thetaGammaLargerThanOne));
> {code}
> This is a different parameter order for the two samplers.
> So the tests enforce the fact that the parameters are swapped between
> Commons Math3 and Commons RNG.
> Changing the actual parameter order would be a change of functionality. So
> this can be fixed by updating the Javadoc and parameter names for the sampler.
> {noformat}
> // Commons RNG
> alpha renamed to theta (scale)
> theta renamed to alpha (shape)
> {noformat}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
