You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Ole Ersoy (JIRA)" <ji...@apache.org> on 2011/03/06 01:45:45 UTC
[jira] Created: (MATH-540)
AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
-----------------------------------------------------------------
Key: MATH-540
URL: https://issues.apache.org/jira/browse/MATH-540
Project: Commons Math
Issue Type: Bug
Affects Versions: 2.1
Reporter: Ole Ersoy
Fix For: 3.0
Attachments: DummyDiscreteDistribution.java, DummyDiscreteDistributionTest.java
The AbstractIntegerDistribution.inverseCumulativeProbability(...) function attempts to decrement the lower bound of discrete distributions to values that go below the lower bound.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (MATH-540)
AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
Posted by "Ole Ersoy (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-540?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003254#comment-13003254 ]
Ole Ersoy commented on MATH-540:
--------------------------------
Ooops - Thanks.
{quote}
...inverse cumulative probability function evaluated at .0001 should be -1, as this is the largest value such that
p(X <= x) <= .0001.
{quote}
It seems to me that users would be better served if it returned 0 and that it is also correct to do so.
In the definition we say "For a random variables X whose values are distributed according to this distribution...".
Suppose the distribution was for a six sided dice. One could assert that the distribution is only defined for the values 1,2,3,4,5,6. In this case the inverseCumulativeDistribution returns 0, but that does not have any meaning. So now developers are forced to define the meaning of 0 for a six sided dice implementation.
In Grad school we were taught the the inverse cumulative distribution is for sampling. So for a six sided dice uniform probabilities less than 1/6 would return 1, less than 2/6 would return 2, etc.
With the current implementation for values less than 1/6 we get 0 which is meaningless, and the only time we get 6 is when the uniform probability argument is 1.
So if someone mistakenly tries to use the inverseCumulativeProbability function for sampling the results are going to be wacked. What is the use case for the inverseCumulativeProbability the way it is right now?
> AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
> -----------------------------------------------------------------
>
> Key: MATH-540
> URL: https://issues.apache.org/jira/browse/MATH-540
> Project: Commons Math
> Issue Type: Bug
> Affects Versions: 2.1
> Reporter: Ole Ersoy
> Fix For: 3.0
>
> Attachments: DummyDiscreteDistribution.java, DummyDiscreteDistributionTest.java
>
>
> The AbstractIntegerDistribution.inverseCumulativeProbability(...) function attempts to decrement the lower bound of discrete distributions to values that go below the lower bound.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Reopened: (MATH-540)
AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-540?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phil Steitz reopened MATH-540:
------------------------------
There is a javadoc bug that needs to be fixed here
> AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
> -----------------------------------------------------------------
>
> Key: MATH-540
> URL: https://issues.apache.org/jira/browse/MATH-540
> Project: Commons Math
> Issue Type: Bug
> Affects Versions: 2.1
> Reporter: Ole Ersoy
> Fix For: 3.0
>
> Attachments: DummyDiscreteDistribution.java, DummyDiscreteDistributionTest.java
>
>
> The AbstractIntegerDistribution.inverseCumulativeProbability(...) function attempts to decrement the lower bound of discrete distributions to values that go below the lower bound.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (MATH-540)
AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
Posted by "Ole Ersoy (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-540?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003414#comment-13003414 ]
Ole Ersoy commented on MATH-540:
--------------------------------
OK - I think it's starting to make more sense to me now. So when implementing sampling we just add one to the value returned by inverseCumulativeDistribution, unless the uniform probability argument is 1?
> AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
> -----------------------------------------------------------------
>
> Key: MATH-540
> URL: https://issues.apache.org/jira/browse/MATH-540
> Project: Commons Math
> Issue Type: Bug
> Affects Versions: 2.1
> Reporter: Ole Ersoy
> Fix For: 3.0
>
> Attachments: DummyDiscreteDistribution.java, DummyDiscreteDistributionTest.java
>
>
> The AbstractIntegerDistribution.inverseCumulativeProbability(...) function attempts to decrement the lower bound of discrete distributions to values that go below the lower bound.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Reopened: (MATH-540)
AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-540?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phil Steitz reopened MATH-540:
------------------------------
Sorry for the noise. I closed the wrong ticket. Still need to fix the javadoc to match behavior and user guide.
> AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
> -----------------------------------------------------------------
>
> Key: MATH-540
> URL: https://issues.apache.org/jira/browse/MATH-540
> Project: Commons Math
> Issue Type: Bug
> Affects Versions: 2.1
> Reporter: Ole Ersoy
> Fix For: 3.0
>
> Attachments: DummyDiscreteDistribution.java, DummyDiscreteDistributionTest.java
>
>
> The AbstractIntegerDistribution.inverseCumulativeProbability(...) function attempts to decrement the lower bound of discrete distributions to values that go below the lower bound.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Resolved] (MATH-540)
AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-540?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phil Steitz resolved MATH-540.
------------------------------
Resolution: Fixed
Javadoc fixed in trunk r1134866
> AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
> -----------------------------------------------------------------
>
> Key: MATH-540
> URL: https://issues.apache.org/jira/browse/MATH-540
> Project: Commons Math
> Issue Type: Bug
> Affects Versions: 2.1
> Reporter: Ole Ersoy
> Fix For: 3.0
>
> Attachments: DummyDiscreteDistribution.java, DummyDiscreteDistributionTest.java
>
>
> The AbstractIntegerDistribution.inverseCumulativeProbability(...) function attempts to decrement the lower bound of discrete distributions to values that go below the lower bound.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (MATH-540)
AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-540?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003107#comment-13003107 ]
Phil Steitz commented on MATH-540:
----------------------------------
I see now that there actually does appear to be an error in the javadoc. The implementation really returns the largest x such that p(X <= x) <= p. In the discrete case, <= matters and I think both inequalities in the javadoc should be changed.
In your example, if the probability distribution vanishes outside 0, 1, 2, 3 and puts .25 mass on each of these values, the inverse cumulative probability function evaluated at .0001 should be -1, as this is the largest value such that
p(X <= x) <= .0001.
If you fix your distribution so that both probability and cumulativeProbability return correct values (rather than throwing NPEs) outside of the mass values, you should get -1 returned.
> AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
> -----------------------------------------------------------------
>
> Key: MATH-540
> URL: https://issues.apache.org/jira/browse/MATH-540
> Project: Commons Math
> Issue Type: Bug
> Affects Versions: 2.1
> Reporter: Ole Ersoy
> Fix For: 3.0
>
> Attachments: DummyDiscreteDistribution.java, DummyDiscreteDistributionTest.java
>
>
> The AbstractIntegerDistribution.inverseCumulativeProbability(...) function attempts to decrement the lower bound of discrete distributions to values that go below the lower bound.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Resolved: (MATH-540)
AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
Posted by "Ole Ersoy (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-540?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Ole Ersoy resolved MATH-540.
----------------------------
Resolution: Not A Problem
> AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
> -----------------------------------------------------------------
>
> Key: MATH-540
> URL: https://issues.apache.org/jira/browse/MATH-540
> Project: Commons Math
> Issue Type: Bug
> Affects Versions: 2.1
> Reporter: Ole Ersoy
> Fix For: 3.0
>
> Attachments: DummyDiscreteDistribution.java, DummyDiscreteDistributionTest.java
>
>
> The AbstractIntegerDistribution.inverseCumulativeProbability(...) function attempts to decrement the lower bound of discrete distributions to values that go below the lower bound.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (MATH-540)
AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-540?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003178#comment-13003178 ]
Phil Steitz commented on MATH-540:
----------------------------------
Reading your last comment a little more carefully, it looks like what you are trying to do is implement sampling. IIUC, something like what you are suggesting should work - you just have an off-by-one problem vis-s-vis the contract of inverse cumulative probabilities as we define them. I would be +1 for adding direct support for sampling from discrete distributions, but we should open a separate ticket for that.
> AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
> -----------------------------------------------------------------
>
> Key: MATH-540
> URL: https://issues.apache.org/jira/browse/MATH-540
> Project: Commons Math
> Issue Type: Bug
> Affects Versions: 2.1
> Reporter: Ole Ersoy
> Fix For: 3.0
>
> Attachments: DummyDiscreteDistribution.java, DummyDiscreteDistributionTest.java
>
>
> The AbstractIntegerDistribution.inverseCumulativeProbability(...) function attempts to decrement the lower bound of discrete distributions to values that go below the lower bound.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Closed: (MATH-540)
AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-540?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phil Steitz closed MATH-540.
----------------------------
Resolution: Not A Problem
I am sorry. I forgot that we had in fact already implemented this in version 2.2. See AbstractIntegerDistribution#sample. The base class implementation delegates to RandomDataImpl#nextInversionDeviate (adding one per the last comment).
> AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
> -----------------------------------------------------------------
>
> Key: MATH-540
> URL: https://issues.apache.org/jira/browse/MATH-540
> Project: Commons Math
> Issue Type: Bug
> Affects Versions: 2.1
> Reporter: Ole Ersoy
> Fix For: 3.0
>
> Attachments: DummyDiscreteDistribution.java, DummyDiscreteDistributionTest.java
>
>
> The AbstractIntegerDistribution.inverseCumulativeProbability(...) function attempts to decrement the lower bound of discrete distributions to values that go below the lower bound.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (MATH-540)
AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-540?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003369#comment-13003369 ]
Phil Steitz commented on MATH-540:
----------------------------------
You have a choice in defining the inverse cum whether to define it the way we have or to use and inf rather than a sup. We can implement sampling using the current impl. We just need to take into account the way the inverse cum is defined in AbstractIntegerDistribution.
> AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
> -----------------------------------------------------------------
>
> Key: MATH-540
> URL: https://issues.apache.org/jira/browse/MATH-540
> Project: Commons Math
> Issue Type: Bug
> Affects Versions: 2.1
> Reporter: Ole Ersoy
> Fix For: 3.0
>
> Attachments: DummyDiscreteDistribution.java, DummyDiscreteDistributionTest.java
>
>
> The AbstractIntegerDistribution.inverseCumulativeProbability(...) function attempts to decrement the lower bound of discrete distributions to values that go below the lower bound.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (MATH-540)
AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
Posted by "Ole Ersoy (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-540?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003103#comment-13003103 ]
Ole Ersoy commented on MATH-540:
--------------------------------
I'm looking at it like this. I have very simple distribution like the one provided (Four sided dice). I'm trying to write a simulation that draws values of x for a a set of uniform 0-1 probabilities. So I'm expecting:
0 When p is less than or equal to 0.25
1 When p is greater than 0.25 but less than or equal to 0.50
2 When p is greater than 0.50 but less than or equal to 0.75
3 When p is greater than 0.75 but less than or equal to 1.0
So for the line
int neverSucceeds = d.inverseCumulativeProbability(0.0001);
I'm really expecting 0 to be returned.
Make sense?
> AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
> -----------------------------------------------------------------
>
> Key: MATH-540
> URL: https://issues.apache.org/jira/browse/MATH-540
> Project: Commons Math
> Issue Type: Bug
> Affects Versions: 2.1
> Reporter: Ole Ersoy
> Fix For: 3.0
>
> Attachments: DummyDiscreteDistribution.java, DummyDiscreteDistributionTest.java
>
>
> The AbstractIntegerDistribution.inverseCumulativeProbability(...) function attempts to decrement the lower bound of discrete distributions to values that go below the lower bound.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (MATH-540)
AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-540?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003097#comment-13003097 ]
Phil Steitz commented on MATH-540:
----------------------------------
I don't think this is a bug. Per the javadoc, the contract for inverse cum is
{code}
/**
* For a random variable {@code X} whose values are distributed according
* to this distribution, this method returns the largest {@code x}, such
* that {@code P(X < x) < p}.
{code}
This implies that if the first non-zero mass point has probability greater than p, the right value to return is one less than that value, which is whet the method will do. Your example distribution throws NPE when trying to compute probabilities outside of its domain of support.
> AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
> -----------------------------------------------------------------
>
> Key: MATH-540
> URL: https://issues.apache.org/jira/browse/MATH-540
> Project: Commons Math
> Issue Type: Bug
> Affects Versions: 2.1
> Reporter: Ole Ersoy
> Fix For: 3.0
>
> Attachments: DummyDiscreteDistribution.java, DummyDiscreteDistributionTest.java
>
>
> The AbstractIntegerDistribution.inverseCumulativeProbability(...) function attempts to decrement the lower bound of discrete distributions to values that go below the lower bound.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (MATH-540)
AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
Posted by "Ole Ersoy (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-540?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003191#comment-13003191 ]
Ole Ersoy commented on MATH-540:
--------------------------------
OK - I'll close this one and open a separate ticket.
> AbstractIntegerDistribution.inverseCumulativeProbability(...) Bug
> -----------------------------------------------------------------
>
> Key: MATH-540
> URL: https://issues.apache.org/jira/browse/MATH-540
> Project: Commons Math
> Issue Type: Bug
> Affects Versions: 2.1
> Reporter: Ole Ersoy
> Fix For: 3.0
>
> Attachments: DummyDiscreteDistribution.java, DummyDiscreteDistributionTest.java
>
>
> The AbstractIntegerDistribution.inverseCumulativeProbability(...) function attempts to decrement the lower bound of discrete distributions to values that go below the lower bound.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira