Uploaded image for project: 'Commons RNG'
  1. Commons RNG
  2. RNG-96

AhrensDieterMarsagliaTsangGammaSampler incorrectly names parameters

VotersWatch issueWatchersLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • 1.3
    • 1.3
    • sampling
    • None

    Description

      The AhrensDieterMarsagliaTsangGammaSampler has parameters alpha and theta.

      Using the naming conventions on Wikipedia Gamma distribution the alpha parameter is also known as the shape and the theta parameter is the scale:

      // Wikipedia
      alpha = shape
      theta = scale
      

      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.

      // Commons RNG
      alpha = scale (wrong)
      theta = shape (wrong)
      

      The unit tests for this sampler does this:

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

      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.

      // Commons RNG
      alpha renamed to theta (scale)
      theta renamed to alpha (shape)
      

      Attachments

        1. dist_density.gnuplot
          1 kB
          Gilles Sadowski
        2. dist_density.sh
          2 kB
          Gilles Sadowski

        Issue Links

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            aherbert Alex Herbert
            aherbert Alex Herbert
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0h
                0h
                Logged:
                Time Spent - 50m
                50m

                Slack

                  Issue deployment