[
https://issues.apache.org/jira/browse/MATH-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13812501#comment-13812501
]
Phil Steitz commented on MATH-1056:
-----------------------------------
Good catch, Sean! I played a little with this today to see if fixing it
allowed me to increase sensitivity in the nextPossionConsistency test in
RandomDataGeneratorTest. I did not succeed, which is unfortunate as this
definitely looks like a bug. I am +1 for committing the patch, but would
really like to get a test that fails before and succeeds after.
The nextPossionConsistency test is a ChiSquare-based test that looks at the
binned distribution of values generated by nextPoission (which is now
implemented in the distribution class itself). When the test was developed, we
had not yet implemented the G test, which may work better for homogeneity
testing in this kind of setting. It might be interesting to experiment with G
tests to get a pre/post fail/fix here. Another thing to dig into is exactly
what generated values for what means will be materially impacted by the bug.
That might allow us to set up a specific G or Chi-square test that will
consistently fail.
> 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)
