[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[
https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13163297#comment-13163297
]
Sébastien Brisard commented on MATH-699:
I've realized that boundary cases ({{p == 0}} and {{p == 1}}) are now handled
correctly: concrete instances of {{AbstractRealDistribution}} no longer need to
override this method, which are removed in rev {{1210756}}.
inverseCumulativeDistribution fails with cumulative distribution having a
plateau
-
Key: MATH-699
URL: https://issues.apache.org/jira/browse/MATH-699
Project: Commons Math
Issue Type: Bug
Affects Versions: 2.2
Reporter: Sébastien Brisard
Assignee: Sébastien Brisard
Priority: Minor
Fix For: 3.0
Attachments: AbstractContinuousDistributionTest.java,
inv-cum-new-impl.zip
This bug report follows MATH-692. The attached unit test fails. As required
by the definition in MATH-692, the lower-bound of the interval on which the
cdf is constant should be returned. This is not so at the moment.
--
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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[
https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13162458#comment-13162458
]
Christian Winter commented on MATH-699:
---
Yes, we can get rid of {{IntegerDistribution.getDomainLowerBound(double)}} and
{{IntegerDistribution.getDomainUpperBound(double)}}. I will do this in the
reimplementation of
{{IntegerDistribution.inverseCumulativeProbability(double)}} based on MATH-692.
inverseCumulativeDistribution fails with cumulative distribution having a
plateau
-
Key: MATH-699
URL: https://issues.apache.org/jira/browse/MATH-699
Project: Commons Math
Issue Type: Bug
Affects Versions: 2.2
Reporter: Sébastien Brisard
Assignee: Sébastien Brisard
Priority: Minor
Fix For: 3.0
Attachments: AbstractContinuousDistributionTest.java,
inv-cum-new-impl.zip
This bug report follows MATH-692. The attached unit test fails. As required
by the definition in MATH-692, the lower-bound of the interval on which the
cdf is constant should be returned. This is not so at the moment.
--
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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[
https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13162169#comment-13162169
]
Sébastien Brisard commented on MATH-699:
New implementation (+ unit tests) committed in {{r1209942}}.
{{getDomainLowerBound(double)}}, {{getDomainUpperBound(double)}} and
{{getInitialDomain(double p)}} have now become superfluous.
inverseCumulativeDistribution fails with cumulative distribution having a
plateau
-
Key: MATH-699
URL: https://issues.apache.org/jira/browse/MATH-699
Project: Commons Math
Issue Type: Bug
Affects Versions: 2.2
Reporter: Sébastien Brisard
Assignee: Sébastien Brisard
Priority: Minor
Fix For: 3.0
Attachments: AbstractContinuousDistributionTest.java,
inv-cum-new-impl.zip
This bug report follows MATH-692. The attached unit test fails. As required
by the definition in MATH-692, the lower-bound of the interval on which the
cdf is constant should be returned. This is not so at the moment.
--
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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[ https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13157310#comment-13157310 ] Phil Steitz commented on MATH-699: -- Wow, we can really improve the code when we put our heads together :) Great observation by Christian - we can switch on plateau detection iff isSupportConnected is false. Otherwise, I would say the new implementation is good to go. I would also be happy to celebrate dropping all of the getXxx(p) methods, which are not needed any more. We can all thank Mikkel, btw, for suggesting a while back that we add the numerical mean, variance and connected support properties. Came in very handy here! inverseCumulativeDistribution fails with cumulative distribution having a plateau - Key: MATH-699 URL: https://issues.apache.org/jira/browse/MATH-699 Project: Commons Math Issue Type: Bug Affects Versions: 3.0 Reporter: Sébastien Brisard Assignee: Sébastien Brisard Priority: Minor Attachments: AbstractContinuousDistributionTest.java, inv-cum-new-impl.zip This bug report follows MATH-692. The attached unit test fails. As required by the definition in MATH-692, the lower-bound of the interval on which the cdf is constant should be returned. This is not so at the moment. -- 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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[
https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13157369#comment-13157369
]
Sébastien Brisard commented on MATH-699:
{quote}
Great observation by Christian - we can switch on plateau detection iff
isSupportConnected is false. Otherwise, I would say the new implementation is
good to go.
{quote}
Yes, it is indeed a very neat way to proceed. I'll make the changes once
Christian's patch for MATH-703 is committed.
{quote}
I would also be happy to celebrate dropping all of the getXxx(p) methods, which
are not needed any more.
{quote}
Good! So I do not need to post on the mailing list on this issue?
{quote}
We can all thank Mikkel, btw, for suggesting a while back that we add the
numerical mean, variance and connected support properties. Came in very handy
here!
{quote}
I would also thank the guy who suggested Chebyshev's inequality (I wonder who
that is ;)). Overall, I personnally enjoyed working on this issue, as it truly
was team work.
Sébastien
inverseCumulativeDistribution fails with cumulative distribution having a
plateau
-
Key: MATH-699
URL: https://issues.apache.org/jira/browse/MATH-699
Project: Commons Math
Issue Type: Bug
Affects Versions: 3.0
Reporter: Sébastien Brisard
Assignee: Sébastien Brisard
Priority: Minor
Attachments: AbstractContinuousDistributionTest.java,
inv-cum-new-impl.zip
This bug report follows MATH-692. The attached unit test fails. As required
by the definition in MATH-692, the lower-bound of the interval on which the
cdf is constant should be returned. This is not so at the moment.
--
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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[ https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13156897#comment-13156897 ] Christian Winter commented on MATH-699: --- Thanks for the patch, Sébastien. There is no need for a plateaux detection flag in the constructor. While dealing with the distribution classes and interfaces, I saw the method isSupportConnected(). This method will tell whether plateaux might occur because a plateau in the cdf corresponds to a gap in the support. inverseCumulativeDistribution fails with cumulative distribution having a plateau - Key: MATH-699 URL: https://issues.apache.org/jira/browse/MATH-699 Project: Commons Math Issue Type: Bug Affects Versions: 3.0 Reporter: Sébastien Brisard Assignee: Sébastien Brisard Priority: Minor Attachments: AbstractContinuousDistributionTest.java, inv-cum-new-impl.zip This bug report follows MATH-692. The attached unit test fails. As required by the definition in MATH-692, the lower-bound of the interval on which the cdf is constant should be returned. This is not so at the moment. -- 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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[
https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13147315#comment-13147315
]
Christian Winter commented on MATH-699:
---
The Javadoc of {{getSupportLowerBound()}}/{{getSupportUpperBound()}} in fact
needs to be sharpened. I didn't see this before. If I didn't overlook anything,
all current implementations already return the best bounds.
Regarding the bracketing issue there are now several ideas (thanks for the
Chebyshev idea, Phil). But I don't have a preference for a particular solution
which can be built from these ideas. Thus, Sébastien, just choose a strategy
which you consider to be beneficial.
inverseCumulativeDistribution fails with cumulative distribution having a
plateau
-
Key: MATH-699
URL: https://issues.apache.org/jira/browse/MATH-699
Project: Commons Math
Issue Type: Bug
Affects Versions: 3.0
Reporter: Sébastien Brisard
Assignee: Sébastien Brisard
Priority: Minor
Attachments: AbstractContinuousDistributionTest.java
This bug report follows MATH-692. The attached unit test fails. As required
by the definition in MATH-692, the lower-bound of the interval on which the
cdf is constant should be returned. This is not so at the moment.
--
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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[
https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13146614#comment-13146614
]
Christian Winter commented on MATH-699:
---
Much discussion has happened here in the recent days. Thanks!
Now I realize that it is hard to get rid of the bracketing step in the case of
unbounded support. I guess a default bracket of
{{[-Double.MAX_VALUE;+Double.MAX_VALUE]}} for {{0p1}} isn't a good idea for
such cases. Maybe we can move the bracketing step into a {{getBracket(double
p)}} method (only {{0p1}} needs to be handled), which makes use of the
support bounds where possible and performs an bracketing algorithm only as last
option. For specific distributions this method can be overridden by providing
brackets through precomputed cdf-values or clever estimations, which removes
using an bracketing algorithm at all. Such a method allows to delete
{{getDomainLowerBound}}/{{getDomainUpperBound}}/{{getInitialDomain}}. However,
this replacement doesn't remove the pain caused by
{{getDomainLowerBound}}/{{getDomainUpperBound}}/{{getInitialDomain}}
completely. It is just a little mitigation to this pain as {{getBracket}}
doesn't have to be overridden (though it should be overridden to increase
performance unless {{inverseCumulativeProbability}} is overridden entirely).
For the plateau issue I unfortunately don't see a way to avoid implementing a
modified solver so that it approximates the solution by moving together {{x0}}
and {{x1}} satisfying {{P(X=x0) p}} and {{P(X=x1) = p}}.
inverseCumulativeDistribution fails with cumulative distribution having a
plateau
-
Key: MATH-699
URL: https://issues.apache.org/jira/browse/MATH-699
Project: Commons Math
Issue Type: Bug
Affects Versions: 3.0
Reporter: Sébastien Brisard
Assignee: Sébastien Brisard
Priority: Minor
Attachments: AbstractContinuousDistributionTest.java
This bug report follows MATH-692. The attached unit test fails. As required
by the definition in MATH-692, the lower-bound of the interval on which the
cdf is constant should be returned. This is not so at the moment.
--
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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[ https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13146627#comment-13146627 ] Christian Winter commented on MATH-699: --- A different idea for solving the bracketing issue: We could solve the inverse cdf calculation by transforming the domain [-infin;;+infin;] to [-1;+1] with arctan. More precisely, we could do the first iterations of the solving step on the transformed domain, and we can go back to original domain as soon as +/-1 aren't interval limits any more. inverseCumulativeDistribution fails with cumulative distribution having a plateau - Key: MATH-699 URL: https://issues.apache.org/jira/browse/MATH-699 Project: Commons Math Issue Type: Bug Affects Versions: 3.0 Reporter: Sébastien Brisard Assignee: Sébastien Brisard Priority: Minor Attachments: AbstractContinuousDistributionTest.java This bug report follows MATH-692. The attached unit test fails. As required by the definition in MATH-692, the lower-bound of the interval on which the cdf is constant should be returned. This is not so at the moment. -- 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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[ https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13146739#comment-13146739 ] Phil Steitz commented on MATH-699: -- Interesting idea. Worth playing with. One more archaeological fact that occurred to me is that the current setup with domain upper / lower bounds used in bracketing was developed before we required either mean or variance from distributions. We now have getNumericalMean and getNumericalVariance. When these both exist and are finite, inequalities such as Chebyshev's might be useful in setting up brackets. inverseCumulativeDistribution fails with cumulative distribution having a plateau - Key: MATH-699 URL: https://issues.apache.org/jira/browse/MATH-699 Project: Commons Math Issue Type: Bug Affects Versions: 3.0 Reporter: Sébastien Brisard Assignee: Sébastien Brisard Priority: Minor Attachments: AbstractContinuousDistributionTest.java This bug report follows MATH-692. The attached unit test fails. As required by the definition in MATH-692, the lower-bound of the interval on which the cdf is constant should be returned. This is not so at the moment. -- 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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[
https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13146814#comment-13146814
]
Sébastien Brisard commented on MATH-699:
{quote}
A different idea for solving the bracketing issue: We could solve the inverse
cdf calculation by transforming the domain [-∞;+∞] to [-1;+1] with arctan. More
precisely, we could do the first iterations of the solving step on the
transformed domain, and we can go back to original domain as soon as +/-1
aren't interval limits any more.
{quote}
Is it not a disguised bracketting step, with a somewhat more clever way of
sampling the reals (as opposed to arithmetic progression)? In this case, maybe
it would be just as simple to perform a bracketting step, with *geometric*
progression (as proposed by Christian), when one of the bounds of the support
is not finite.
{quote}
only 0p1 needs to be handled
{quote}
I guess you mean by that that what should be done is
* return {{getSupportLowerBound()}} when {{p == 0}},
* return {{getSupportUpperBound()}} when {{p == 1}}.
This means that the contract of
{{getSupportLowerBound()}}/{{getSupportUpperBound()}} *requires* that the
returned values are actually the *best* bounds on the support. While I have no
problem with this sound requirement, I don't think this is currently stated
clearly in the Javadoc, but I'm no native english speaker...
inverseCumulativeDistribution fails with cumulative distribution having a
plateau
-
Key: MATH-699
URL: https://issues.apache.org/jira/browse/MATH-699
Project: Commons Math
Issue Type: Bug
Affects Versions: 3.0
Reporter: Sébastien Brisard
Assignee: Sébastien Brisard
Priority: Minor
Attachments: AbstractContinuousDistributionTest.java
This bug report follows MATH-692. The attached unit test fails. As required
by the definition in MATH-692, the lower-bound of the interval on which the
cdf is constant should be returned. This is not so at the moment.
--
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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[ https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13145544#comment-13145544 ] Phil Steitz commented on MATH-699: -- My inclination would be to keep the implementation in the base class as simple as possible, documenting what it does and pushing the responsibility for dealing with plateaus in the distribution to the implementations that have these. I don't think any of the currently implemented real distributions have this problem. Correct? The invariant you are proposing for when to return infinities for domain lower and upper bounds would make sense if these were intended to be the bounds of support for the distribution, but this is not what these properties represent. They are initial guesses for where to start when trying to bracket a root. That means they have to be values that can be fed into the cumulative probability function. Have you convinced yourself that we do not need to bracket a root at the beginning? What bounds would we then provide to the solver? The code you reference is trying to do the bracketing, starting with the domain upper and lower bounds as initial guesses. Remember to consider convergence problems when the actual parameter to inverse cum is close to or exactly equal to 0 or 1. Per the comment in the code above the test that uses (or should use) the function value absolute accuracy, if the distribution has bounded support and the argument is 0 or 1, bracketing will fail. inverseCumulativeDistribution fails with cumulative distribution having a plateau - Key: MATH-699 URL: https://issues.apache.org/jira/browse/MATH-699 Project: Commons Math Issue Type: Bug Affects Versions: 3.0 Reporter: Sébastien Brisard Assignee: Sébastien Brisard Priority: Minor Attachments: AbstractContinuousDistributionTest.java This bug report follows MATH-692. The attached unit test fails. As required by the definition in MATH-692, the lower-bound of the interval on which the cdf is constant should be returned. This is not so at the moment. -- 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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[
https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13145718#comment-13145718
]
Sébastien Brisard commented on MATH-699:
{quote}
I don't think any of the currently implemented real distributions have this
problem. Correct?
{quote}
Maybe I misunderstood, but in MATH-692 asked for a more precise definition of
{{inverseCumulativeProbability}} as inf{x in R | P(X=x) = p}. If we exclude
distribution with plateaus, I think that the current implementation is
satisfactory (but for the use of the wrong tolerance I've already pointed out).
But it was agreed that this implementation should be made more robust. So what
was it exactly that needed improvement? What do you want me to do on this
method?
{quote}
They are initial guesses for where to start when trying to bracket a root. That
means they have to be values that can be fed into the cumulative probability
function.
{quote}
I'm aware of that, but the current solver does fail when
{{inverseCumulativeProbability(0)}} should return -inf, or
{{inverseCumulativeProbability(1)}} should return +inf. See for example the
implementation of {{NormalDistributionImpl}}.
{code:java}
public double inverseCumulativeProbability(final double p)
{
if (p == 0) {
return Double.NEGATIVE_INFINITY;
}
if (p == 1) {
return Double.POSITIVE_INFINITY;
}
return super.inverseCumulativeProbability(p);
}
{code}
So currently, people who want to implement a distribution must be aware of the
fact that the default implementation of {{inverseCumulativeProbability}} *must*
be overriden. This rather unusual fact should be made clear in the Javadoc,
unless a workaround can be thought of. I agree the one I proposed was far from
perfect.
{quote}
Remember to consider convergence problems when the actual parameter to inverse
cum is close to or exactly equal to 0 or 1.
{quote}
Thank you for pointing this out earlier. I do keep this important point in
mind. But again, if we do not widen the scope of this method, I don't see what
is required of me (appart from some cosmetic alterations to the Javadoc).
inverseCumulativeDistribution fails with cumulative distribution having a
plateau
-
Key: MATH-699
URL: https://issues.apache.org/jira/browse/MATH-699
Project: Commons Math
Issue Type: Bug
Affects Versions: 3.0
Reporter: Sébastien Brisard
Assignee: Sébastien Brisard
Priority: Minor
Attachments: AbstractContinuousDistributionTest.java
This bug report follows MATH-692. The attached unit test fails. As required
by the definition in MATH-692, the lower-bound of the interval on which the
cdf is constant should be returned. This is not so at the moment.
--
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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[ https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13146060#comment-13146060 ] Phil Steitz commented on MATH-699: -- I am sorry, Sebastien. I am not being very helpful here. As I dig deeper into the code and archaeology, I am having a hard time seeing how we can really improve the default impl without knowledge of the underlying distribution. I agree with you that we should clearly document its limitations, though, and fix the javadoc contracts everywhere to be correct and consistent. The default impl was never really intended to be universal - just a simple solver-based impl that would work for well-behaved distributions. The normal distribution example above points to a slight improvement that could now be done. When that code was written, we had yet to define the supportLowerBound and supportUpperBound properties. Now that we have those, the test in the normal dist case could be moved up into the default impl, using the getters on the distribution for the return values. By all means if you can find a way to deal with plateaus or otherwise improve robustness of the default impl, go for it. In particular, if you can come up with a better way to set up the solver, possibly eliminating the domainLower/upperBound methods, that would be great. It has always seemed a little ugly to me that we had to implement these methods for every distribution just so we could get initial guesses for the inverse cum solver. inverseCumulativeDistribution fails with cumulative distribution having a plateau - Key: MATH-699 URL: https://issues.apache.org/jira/browse/MATH-699 Project: Commons Math Issue Type: Bug Affects Versions: 3.0 Reporter: Sébastien Brisard Assignee: Sébastien Brisard Priority: Minor Attachments: AbstractContinuousDistributionTest.java This bug report follows MATH-692. The attached unit test fails. As required by the definition in MATH-692, the lower-bound of the interval on which the cdf is constant should be returned. This is not so at the moment. -- 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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[
https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13146108#comment-13146108
]
Sébastien Brisard commented on MATH-699:
I agree with you: however careful we are, the default implementation will never
be perfect, and it should be clearly stated in the javadoc. I see (but haven't
dug into it yet) there is an abstract test for continuous distributions. I'll
try and make sure that most possible cases of failure of
{{inverseCumulativeProbability}} are tested in this abstract framework, so that
users could be *strongly* encouraged to implement this test for their very
specific distribution.
I do think we can widen a little bit the scope of the default implementation. I
actually worked towards eliminating the bracketing step, but you convinced me
that it should be the other way round: can we eliminate the
domainLower/upperBound methods (which I agree are very efficient, but a little
bit of a pain in the neck...).
Here is an idea I'm going to check (keeping in mind the accuracy issues you've
already pointed at)
* for distributions with bounded support: maybe the bounds (possibly shifted if
the bounds are not inclusive) of the support itself could be used as an initial
bracketting range.
* for distributions with unbounded support: start from *any* (? or a better
guess? average value?) point in the support, and do bracketting. However, I do
agree with Christian, we should use geometric progressions in this case,
instead of arithmetic progressions. This would probably lead to a very large
interval, but it would provide us an interval more quickly, and the solver
itself would probably be quite good at narrowing it very quickly. I can try
some monitoring on this issue.
As for plateaus, I think what I'm currently working on is not too inefficient.
I'm sure it's not *the* answer (which is, anyway, 42 :)), but I'm struggling to
fail this algo... Don't worry, I'll break it. I think that what I've done
entails very little additional cost (maybe one more evaluation of
cumulativeProbability) for distributions *without* plateau, but I need to check
that (we do not want to pay a heavy price for plateaus, since most of our
distributions do not have one, as you rightly pointed out).
I would be grateful for any idea on these topics, and will keep you informed.
All the best for now,
Sébastien
inverseCumulativeDistribution fails with cumulative distribution having a
plateau
-
Key: MATH-699
URL: https://issues.apache.org/jira/browse/MATH-699
Project: Commons Math
Issue Type: Bug
Affects Versions: 3.0
Reporter: Sébastien Brisard
Assignee: Sébastien Brisard
Priority: Minor
Attachments: AbstractContinuousDistributionTest.java
This bug report follows MATH-692. The attached unit test fails. As required
by the definition in MATH-692, the lower-bound of the interval on which the
cdf is constant should be returned. This is not so at the moment.
--
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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[
https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13144927#comment-13144927
]
Sébastien Brisard commented on MATH-699:
I think Christian has a point here. The bracketing step is superfluous. I'll
propose a new impl getting rid of it, so that you can decide whether or not to
restore it.
However, we cannot forbit getDomainLowerBound() and getDomainUpperBound() to
return {{Double.POSITIVE_INFINITY}} (for {{p == 1}}) or
{{Double.NEGATIVE_INFINITY}} (for {{p == 0}}). I will prepare something along
these lines.
inverseCumulativeDistribution fails with cumulative distribution having a
plateau
-
Key: MATH-699
URL: https://issues.apache.org/jira/browse/MATH-699
Project: Commons Math
Issue Type: Bug
Affects Versions: 3.0
Reporter: Sébastien Brisard
Assignee: Sébastien Brisard
Priority: Minor
Attachments: AbstractContinuousDistributionTest.java
This bug report follows MATH-692. The attached unit test fails. As required
by the definition in MATH-692, the lower-bound of the interval on which the
cdf is constant should be returned. This is not so at the moment.
--
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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[ https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13144933#comment-13144933 ] Phil Steitz commented on MATH-699: -- Be careful eliminating the bracketing step. IIRC it is in there to resolve a bug involving some corner case. In theory, the unit tests should pick up any regressions, but it would be a good idea to review the commit logs and issue reports for this class before ripping out the bracketing step. inverseCumulativeDistribution fails with cumulative distribution having a plateau - Key: MATH-699 URL: https://issues.apache.org/jira/browse/MATH-699 Project: Commons Math Issue Type: Bug Affects Versions: 3.0 Reporter: Sébastien Brisard Assignee: Sébastien Brisard Priority: Minor Attachments: AbstractContinuousDistributionTest.java This bug report follows MATH-692. The attached unit test fails. As required by the definition in MATH-692, the lower-bound of the interval on which the cdf is constant should be returned. This is not so at the moment. -- 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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[ https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13144939#comment-13144939 ] Sébastien Brisard commented on MATH-699: OK, thanks for the tip. I will do so. Sébastien inverseCumulativeDistribution fails with cumulative distribution having a plateau - Key: MATH-699 URL: https://issues.apache.org/jira/browse/MATH-699 Project: Commons Math Issue Type: Bug Affects Versions: 3.0 Reporter: Sébastien Brisard Assignee: Sébastien Brisard Priority: Minor Attachments: AbstractContinuousDistributionTest.java This bug report follows MATH-692. The attached unit test fails. As required by the definition in MATH-692, the lower-bound of the interval on which the cdf is constant should be returned. This is not so at the moment. -- 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
Re: [jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
Le 06/11/2011 07:34, Sébastien Brisard (Commented) (JIRA) a écrit : [ https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13144939#comment-13144939 ] Sébastien Brisard commented on MATH-699: OK, thanks for the tip. I will do so. You can also identify a which revision a line was last changed by using svn blame. With luck, the last change will be a meaningful information. Luc Sébastien inverseCumulativeDistribution fails with cumulative distribution having a plateau - Key: MATH-699 URL: https://issues.apache.org/jira/browse/MATH-699 Project: Commons Math Issue Type: Bug Affects Versions: 3.0 Reporter: Sébastien Brisard Assignee: Sébastien Brisard Priority: Minor Attachments: AbstractContinuousDistributionTest.java This bug report follows MATH-692. The attached unit test fails. As required by the definition in MATH-692, the lower-bound of the interval on which the cdf is constant should be returned. This is not so at the moment. -- 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
Re: [jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
You can also identify a which revision a line was last changed by using svn blame. With luck, the last change will be a meaningful information. Luc That's a good one too, thanks Luc. I really need to give read the SVN doc on my TODO list top priority... By the way, I like the choice of the blame keyword...
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[
https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13145211#comment-13145211
]
Sébastien Brisard commented on MATH-699:
Hi,
I thought I would keep you up to date with the problems I'm facing at the
moment.
First of all, I see no smart way to check that for sure,
{{inverseCumulativeProbability(0)}} (resp. {{inverseCumulativeProbability(1)}})
should return {{Double.NEGATIVE_INFINITY}} (resp.
{{Double.POSITIVE_INFINITY}}). What could be done would be to test for a
neighbouring value, and check that the cumulative probability is slightly above
zero (resp. below one). But this makes no sense, because the only neighbouring
value which would make sense would be {{+/-Double.MAX_VALUE}}, and this surely
return 0.0 or 1.0. So this is my first problem.
The way I see things is as follows: users might have no clue about the inverse
cumulative probability, but they *must* know the values of this inverse for p =
0 and p = 1. So I would suggest to change the contract of
{{getInitialDomain(p)}}. I would make clear in the javadoc that
{{getInitialDomain(p)}} should return {{Double.NEGATIVE_INFINITY}} *if, and
only if* {{inverseCumulativeProbability(0) == Double.NEGATIVE_INFINITY}}.
Similarly, {{getInitialDomain(p)}} should return {{Double.POSITIVE_INFINITY}}
*if, and only if* {{inverseCumulativeProbability(1) ==
Double.POSITIVE_INFINITY}}.
My second problem is to check for the presence of plateaux, consistently with
finite precision. Here is my initial idea. I first define the two absolute
accuracies
* {{dx = getSolverAbsoluteAccuracy()}},
* {{dp = getSolverFunctionValueAccuracy()}}.
Then, if {{x}} is the root found by the solver: we do have
{{cumulativeProbability( x ) == p}} (within a specified accuracy). The problem
is to check whether there is a *smaller* value which also satisfies this
requirement (in which case, the smallest such value must be returned).
My initial test was {{cumulativeProbability(x - dx) == p}}. Then, I tried
{{FastMath.abs(cumulativeProbability(x - dx) - p) = dp}}. Although more
consistent with finite precision, this is not fully satisfactory, because in
simple cases (where there is no plateau), it might lead to the solver moving to
a somewhat less good point. At the very least, it would lead to additional
iterations... to finally get back to the initial point {{x}}.
Then, I thought of checking for *exact* nullity of the pdf at x. The problem is
that the pdf might have discontinuities, in which case, this simple test might
fail (if {{x}} turns out to be the higher-end of the plateau, and there is a
slope discontinuity here).
So, I'm now back to this test: {{cumulativeProbability(x - dx) ==
cumulativeProbability( x )}}. Please note
* *exact* equality test,
* I'm no longer testing for equality with the target value {{p}}, but with its
estimate {{cumulativeProbability( x )}}.
I would be grateful for some feedback on these issues. Also, I think it is
clear that
* my code will require careful reviewers!!!
* it won't be completely fool-proof.
inverseCumulativeDistribution fails with cumulative distribution having a
plateau
-
Key: MATH-699
URL: https://issues.apache.org/jira/browse/MATH-699
Project: Commons Math
Issue Type: Bug
Affects Versions: 3.0
Reporter: Sébastien Brisard
Assignee: Sébastien Brisard
Priority: Minor
Attachments: AbstractContinuousDistributionTest.java
This bug report follows MATH-692. The attached unit test fails. As required
by the definition in MATH-692, the lower-bound of the interval on which the
cdf is constant should be returned. This is not so at the moment.
--
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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[ https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13144740#comment-13144740 ] Phil Steitz commented on MATH-699: -- I agree that the wrong solver property is being used. I would say yes, add the property with a reasonable default. inverseCumulativeDistribution fails with cumulative distribution having a plateau - Key: MATH-699 URL: https://issues.apache.org/jira/browse/MATH-699 Project: Commons Math Issue Type: Bug Affects Versions: 3.0 Reporter: Sébastien Brisard Assignee: Sébastien Brisard Priority: Minor Attachments: AbstractContinuousDistributionTest.java This bug report follows MATH-692. The attached unit test fails. As required by the definition in MATH-692, the lower-bound of the interval on which the cdf is constant should be returned. This is not so at the moment. -- 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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[ https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13144761#comment-13144761 ] Christian Winter commented on MATH-699: --- I also agree that the mentioned code lines use the wrong property. But I'm not sure whether the bracketing step is necessary and efficient at all. Maybe it's better to pass lowerBound and upperBound directly to the solving step because the solver will shrink the interval efficiently. The bracketing algorithm, however, is very inefficient in expanding the interval around the initial point to a bracket (At least the current implementation is inefficient as it makes linear steps. Geometrical steps would be better for distribution functions, but valid brackets might be missed for non-monotonic functions.). The only problem I see for the solver is if lowerBound or upperBound is infinite. The JavaDoc of getDomainLowerBound() and getDomainUpperBound() just could mention that an implementation must return a finite value. I'm fine with protected access to the solver. inverseCumulativeDistribution fails with cumulative distribution having a plateau - Key: MATH-699 URL: https://issues.apache.org/jira/browse/MATH-699 Project: Commons Math Issue Type: Bug Affects Versions: 3.0 Reporter: Sébastien Brisard Assignee: Sébastien Brisard Priority: Minor Attachments: AbstractContinuousDistributionTest.java This bug report follows MATH-692. The attached unit test fails. As required by the definition in MATH-692, the lower-bound of the interval on which the cdf is constant should be returned. This is not so at the moment. -- 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
[jira] [Commented] (MATH-699) inverseCumulativeDistribution fails with cumulative distribution having a plateau
[
https://issues.apache.org/jira/browse/MATH-699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13144256#comment-13144256
]
Sébastien Brisard commented on MATH-699:
h4. Should this be considered as an error?
Please have a look to lines
[103-108|http://commons.apache.org/math/xref/org/apache/commons/math/distribution/AbstractContinuousDistribution.html#103]
of {{AbstractContinuousDistribution}}.
I think the logics is flawed, I have the feeling that
{{rootFindingFunction.value(lowerBound)}} should be compared with
{{solver.getFunctionValueAccuracy()}}, and not with
{{getSolverAbsoluteAccuracy()}}. The problem is that
{{AbstractContinuousDistribution}} should then have a method called
{{getSolverFunctionValueAccuracy()}}. Should I add one?
Thanks for your comments,
Sébastien
inverseCumulativeDistribution fails with cumulative distribution having a
plateau
-
Key: MATH-699
URL: https://issues.apache.org/jira/browse/MATH-699
Project: Commons Math
Issue Type: Bug
Affects Versions: 3.0
Reporter: Sébastien Brisard
Assignee: Sébastien Brisard
Priority: Minor
Attachments: AbstractContinuousDistributionTest.java
This bug report follows MATH-692. The attached unit test fails. As required
by the definition in MATH-692, the lower-bound of the interval on which the
cdf is constant should be returned. This is not so at the moment.
--
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
