commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Neidhart (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (MATH-1056) Small error in PoissonDistribution.nextPoisson() algorithm
Date Fri, 08 Nov 2013 16:15:19 GMT

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

Mime
View raw message