[
https://issues.apache.org/jira/browse/MATH-385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12935588#action_12935588
]
Phil Steitz commented on MATH-385:
----------------------------------
Second patch looks good. Couple of small nits:
1. In hypergeometric mean calculation you use FastMath.pow(*,2) and in Pascal
you use Math.pow(*,2). Both of these would better be replaced by just
multiplying * by itself.
2. We have an unavoidably smelly situation with Poisson support upper bound. I
guess I agree with Integer.MAX_VALUE; but in that case
isSupportUpperBoundInclusive should actually return true. I would change the
javadoc to
{code}
* The upper bound of the support is positive infinity, regardless of the
parameter values.
* There is no integer infinity, so this method returns
<code>Integer.MAX_VALUE</code> and
* {...@link #isSupportUpperBoundInclusive()} returns <code>true</code>
{code}
If others feel strongly that we should return false for
isSupportUpperBoundInclusive, I am OK with that. Smelly either way.
Regarding tests, I would just add some tests using distributions with known
characteristics and verify that the results are correct. Also verify that when
parameters change, the characteristics also change. Adding R verificiation
tests would be good as well.
> Characteristic (support, mean, variance, ...) on Distributions
> --------------------------------------------------------------
>
> Key: MATH-385
> URL: https://issues.apache.org/jira/browse/MATH-385
> Project: Commons Math
> Issue Type: New Feature
> Reporter: Mikkel Meyer Andersen
> Fix For: 2.2
>
> Attachments: MATH385-PATCH1, MATH385-PATCH2
>
> Original Estimate: 5h
> Remaining Estimate: 5h
>
> I wish that the Distributions could contain some characteristics. For example
> support, mean, and variance.
> Support:
> AbstractContinuousDistribution and AbstractIntegerDistribution should have
> double getSupport{Lower, Upper}Bound() and int getSupport{Lower,
> Upper}Bound(), respectively. Also methods a la boolean isSupport{Lower,
> Upper}BoundInclusive() on AbstractContinuousDistribution should reflect if
> the support is open of closed. In practise the implemented distributions are
> easy since the support for all continuous distributions are real intervals
> (connected sets), and the support for all the discrete distributions are
> connected integer sets. This means that the lower and upper bound (together
> with isSupport{Lower, Upper}BoundInclusive() on
> AbstractContinuousDistribution because it is not needed on the discrete
> distributions because of their nature) are sufficient for determine the
> support.
> Mean and variance:
> double get{Mean, Variance}() should be on AbstractDistribution.
> With such characteristic an invalidateParameters-method might come in handy
> because they often depend on the parameters. The characteristics should not
> be calculated before the first time they are get'ted, and when calculated,
> they should be saved for later use. When parameters change, an
> invalidateParameters-method should be called to force the characteristics to
> be recalculated.
> Values such as Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY, and
> Double.NaN should be used where appropriate.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
