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

Thomas Neidhart commented on MATH-1056:
---------------------------------------

I did take a look at this myself, and the introduced error is so small that it 
is very difficult to make a reasonable test that outlines this.

Due to the fact that the wrong code is only executed when a PoissonDistribution 
with mean >= 40 is used, the error is <= 3e-3. Now when looking at the 
iteration that constructs the sample, it looks at random numbers [0, 1) and 
does different things dependent on whether the random number is smaller or 
larger than some of these pre-computed values. I did not do further analysis of 
the expected failure from such a small difference, but it must be *very* small 
compared to an ideal poisson distribution.

btw. we have there several variables that are calculated each time 
nextPoisson() is called, although they are fixed with the given mean (which is 
immutable for the distribution), thus could be cached to improve performance.

> Small error in PoissonDistribution.nextPoisson() algorithm
> ----------------------------------------------------------
>
>                 Key: MATH-1056
>                 URL: https://issues.apache.org/jira/browse/MATH-1056
>             Project: Commons Math
>          Issue Type: Bug
>    Affects Versions: 3.2
>            Reporter: Sean Owen
>            Priority: Minor
>         Attachments: MATH-1056.patch
>
>
> Here's a tiny bug I noticed via static inspection, since it flagged the 
> integer division. PoissonDistribution.java:325 says:
> {code:java}
> final double a1 = FastMath.sqrt(FastMath.PI * twolpd) * FastMath.exp(1 / 8 * 
> lambda);
> {code}
> The "1 / 8 * lambda" is evidently incorrect, since this will always evaluate 
> to 0. I rechecked the original algorithm 
> (http://luc.devroye.org/devroye-poisson.pdf) and it should instead be:
> {code:java}
> final double a1 = FastMath.sqrt(FastMath.PI * twolpd) * FastMath.exp(1 / (8 * 
> lambda));
> {code}
> (lambda is a double so there is no int division issue.) This matches a later 
> expression.
> I'm not sure how to evaluate the effect of the bug. Better to be correct of 
> course; it may never have made much practical difference.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Reply via email to