On 8/4/12 10:57 AM, Gilles Sadowski wrote:
> Hello.
>
> Referring to this failed test (cf. messages from Continuum):
> ---CUT---
> org.apache.commons.math3.exception.NumberIsTooLargeException: lower bound
> (65) must be strictly less than upper bound (65)
> at
> org.apache.commons.math3.distribution.UniformIntegerDistribution.<init>(UniformIntegerDistribution.java:73)
> at
> org.apache.commons.math3.distribution.UniformIntegerDistribution.<init>(UniformIntegerDistribution.java:53)
> at
> org.apache.commons.math3.stat.descriptive.AggregateSummaryStatisticsTest.generatePartition(AggregateSummaryStatisticsTest.java:275)
> at
> org.apache.commons.math3.stat.descriptive.AggregateSummaryStatisticsTest.testAggregationConsistency(AggregateSummaryStatisticsTest.java:89)
>
>
> It is due to a precondition check while creating the
> "UniformIntegerDistribution" instance:
> ---CUT---
> if (lower >= upper) {
> throw new NumberIsTooLargeException(
> LocalizedFormats.LOWER_BOUND_NOT_BELOW_UPPER_BOUND,
> lower, upper, false);
> }
> ---CUT---
>
> The test referred to above was using this code (before I changed it use a
> "UniformIntegerDistribution" instance):
> ---CUT---
> final int next = (i == 4 || cur == length - 1) ? length - 1 :
> randomData.nextInt(cur, length - 1);
> ---CUT---
>
> It is now (after the change):
> ---CUT---
> final IntegerDistribution partitionPoint = new
> UniformIntegerDistribution(cur, length - 1);
> final int next = (i == 4 || cur == length - 1) ? length - 1 :
> partitionPoint.sample();
> ---CUT---
>
> Thus, AFAIK, the failure did not appear before because there was no
> precondition enforcement in "nextInt".
>
> The question is: Was the code in the test correct (in allowing the same
> value for both bounds?
> * In the negative, how to change it?
> * The affirmative would mean that the precondition check in
> "UniformIntegerDistribution" should be relaxed to allow equal bounds.
> Does this make sense?
> If so, can we change it now, or is it forbidden in order to stay
> backwards compatible?
Your analysis above is correct. The failure after the change is due
to the fact that post-change the distribution is instantiated before
the bounds check. I changed the test to fix this. Both the
randomData nextInt and the UniformIntegerDistribution constructor
now forbid the degenerate case where there is only one point in the
domain. In retrospect, I guess it would have probably been better
to allow this degenerate case. Unfortunately, this would be an
incompatible change, so will have to wait until 4.0 if we want to do it.
The original code above illustrates the convenience of being able to
just make direct calls to randomData.nextXxx, which is why this
class exists ;)
Phil
>
>
> Regards,
> Gilles
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
