[ 
https://issues.apache.org/jira/browse/RNG-96?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16826039#comment-16826039
 ] 

Gilles commented on RNG-96:
---------------------------

{quote}So should I [...] ?
{quote}
Given the low risk of breaking [user 
codes|https://mvnrepository.com/artifact/org.apache.commons/commons-rng-sampling]
 (and because either option can be breaking user codes), I'd favour the cleaner 
fix (i.e. change parameter order _and_ name in order to match the reference).  
Functionality breaking announcement should prominently appear in the release 
notes.
 [Note: I didn't check whether the two libraries that currently depend on 
{{commons-rng-sampling}} actually use 
{{AhrensDieterMarsagliaTsangGammaSampler}}.]

Class 
[{{GammaDistribution}}|https://gitbox.apache.org/repos/asf?p=commons-statistics.git;a=blob;f=commons-statistics-distribution/src/main/java/org/apache/commons/statistics/distribution/GammaDistribution.java#l97]
 in "Commons Statistics" should also be fixed.

> 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)

Reply via email to