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

Thomas Neidhart edited comment on MATH-644 at 12/15/11 8:53 AM:
----------------------------------------------------------------

I digged a bit into the problem. The HypergeometricDistribution calculates the 
probability for a given x using the following formula:

{code:java}
private double probability(int n, int m, int k, int x) {
  return FastMath.exp(ArithmeticUtils.binomialCoefficientLog(m, x) +
         ArithmeticUtils.binomialCoefficientLog(n - m, k - x) -
         ArithmeticUtils.binomialCoefficientLog(n, k));
}
{code}

Thus it transforms the binomial coefficients to a logarithmic scale in order to 
cope with the possibly large results (and to be able to compute the bincoeff at 
all). But, imho, the reverse transformation is broken, as it does not take any 
scaling into account. As the coefficients get larger (e.g. due to a large n), 
the differences between the terms will become smaller in log scale, and thus 
incorrectly transformed back to linear scale. Taking scaling into account, the 
exp function will most likely fail for large n.

I have created a simple test to illustrate the problem, the t{x} correspond to 
the binomial coeff terms from the formula, diff is the input the the exp 
function. This loop simulates an increasing n, and the expectation is that the 
result should get smaller with increasing n:

{code}
t1=0.0, t2=4547.288942497606, t3=4770.9627189150215, diff=-223.67377641741587, 
result=7.23957639711833E-98
t1=0.0, t2=12183.221706275828, t3=12186.968419291079, diff=-3.7467130152508616, 
result=0.023595175513309037
t1=0.0, t2=13444.672093808651, t3=13446.561352727935, diff=-1.8892589192837477, 
result=0.15118380673528464
t1=0.0, t2=14186.229425843805, t3=14187.492505971342, diff=-1.2630801275372505, 
result=0.2827816800804864
t1=0.0, t2=14713.395226772875, t3=14714.343882929534, diff=-0.9486561566591263, 
result=0.3872610921706871
t1=0.0, t2=15122.726785860956, t3=15123.486358374357, diff=-0.7595725134015083, 
result=0.46786639087791215
t1=0.0, t2=15457.396298892796, t3=15458.029636271298, diff=-0.6333373785018921, 
result=0.5308173033122051
t1=0.0, t2=15740.484181590378, t3=15741.027263000607, diff=-0.5430814102292061, 
result=0.5809553297318205
t1=0.0, t2=15985.787659011781, t3=15986.263000234962, 
diff=-0.47534122318029404, result=0.6216728910682101
t1=0.0, t2=16202.21559868753, t3=16202.638224512339, diff=-0.4226258248090744, 
result=0.6553237931479635
t1=0.0, t2=16395.855738580227, t3=16396.236174091697, diff=-0.380435511469841, 
result=0.6835636445695273
{code}

hmm, after some second thoughts, I am not sure if the analysis is correct, and 
the problem is hidden somewhere else.
                
      was (Author: tn):
    I digged a bit into the problem. The HypergeometricDistribution calculates 
the probability for a given x using the following formula:

{code:java}
private double probability(int n, int m, int k, int x) {
  return FastMath.exp(ArithmeticUtils.binomialCoefficientLog(m, x) +
         ArithmeticUtils.binomialCoefficientLog(n - m, k - x) -
         ArithmeticUtils.binomialCoefficientLog(n, k));
}
{code}

Thus it transforms the binomial coefficients to a logarithmic scale in order to 
cope with the possibly large results (and to be able to compute the bincoeff at 
all). But, imho, the reverse transformation is broken, as it does not take any 
scaling into account. As the coefficients get larger (e.g. due to a large n), 
the differences between the terms will become smaller in log scale, and thus 
incorrectly transformed back to linear scale. Taking scaling into account, the 
exp function will most likely fail for large n.

I have created a simple test to illustrate the problem, the t{x} correspond to 
the binomial coeff terms from the formula, diff is the input the the exp 
function. This loop simulates an increasing n, and the expectation is that the 
result should get smaller with increasing n:

{code}
t1=0.0, t2=4547.288942497606, t3=4770.9627189150215, diff=-223.67377641741587, 
result=7.23957639711833E-98
t1=0.0, t2=12183.221706275828, t3=12186.968419291079, diff=-3.7467130152508616, 
result=0.023595175513309037
t1=0.0, t2=13444.672093808651, t3=13446.561352727935, diff=-1.8892589192837477, 
result=0.15118380673528464
t1=0.0, t2=14186.229425843805, t3=14187.492505971342, diff=-1.2630801275372505, 
result=0.2827816800804864
t1=0.0, t2=14713.395226772875, t3=14714.343882929534, diff=-0.9486561566591263, 
result=0.3872610921706871
t1=0.0, t2=15122.726785860956, t3=15123.486358374357, diff=-0.7595725134015083, 
result=0.46786639087791215
t1=0.0, t2=15457.396298892796, t3=15458.029636271298, diff=-0.6333373785018921, 
result=0.5308173033122051
t1=0.0, t2=15740.484181590378, t3=15741.027263000607, diff=-0.5430814102292061, 
result=0.5809553297318205
t1=0.0, t2=15985.787659011781, t3=15986.263000234962, 
diff=-0.47534122318029404, result=0.6216728910682101
t1=0.0, t2=16202.21559868753, t3=16202.638224512339, diff=-0.4226258248090744, 
result=0.6553237931479635
t1=0.0, t2=16395.855738580227, t3=16396.236174091697, diff=-0.380435511469841, 
result=0.6835636445695273
{code}
                  
> for the class of hyper-geometric distribution, for some number the method 
> "upperCumulativeProbability" return a probability greater than 1 which is 
> impossible.  
> -----------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: MATH-644
>                 URL: https://issues.apache.org/jira/browse/MATH-644
>             Project: Commons Math
>          Issue Type: Bug
>    Affects Versions: 2.2
>            Reporter: marzieh
>            Priority: Minor
>              Labels: hypergeometric, probability
>             Fix For: 2.2
>
>
> In windows 7, I used common.Math library. I used class 
> "HypergeometricDistributionImpl" and method "upperCumulativeProbability" of 
> zero for distribution and the return value is larget than 1. the following 
> code is working error. 
> HypergeometricDistributionImpl u = new 
> HypergeometricDistributionImpl(14761461, 1035 ,1841 );
> System.out.println(u.upperCumulativeProbability(0))
> Thanks

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to