[
https://issues.apache.org/jira/browse/RNG-145?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17363112#comment-17363112
]
Alex Herbert commented on RNG-145:
----------------------------------
The change to the sampler was valid but the test used {{>=}} and {{<=}} to test
the bound. When updated to use {{<}} and {{>}} it fails due to floating point
inaccuracy. Eliminating the sampler, the following test fails:
{code:java}
@Test
public void testOpenInterval() {
final double low = 3.18;
final double high = 5.23;
final double u = 0x1.0p-53d;
final double value = u * high + (1 - u) * low;
Assert.assertTrue("Value not in range: " + value, value > low && value <
high);
}
{code}
The range between 5.23 - 3.18 = 2.05. The precision of a double is not enough
to express a value that is the fraction 2^-53 of the range between the two
values. The result is rounded to 3.18.
A correct implementation will require creating the value and then repeating if
the bounds are reached. I've updated the code to use this method (commit
2cfea0baac98f65ce2145b91575847a2d106028e):
{code:java}
@Override
public double sample() {
final double x = super.sample();
// Due to rounding using a variate u in the open interval (0,1) with the
original
// algorithm may generate a value at the bound. Thus the bound is
explicitly tested
// and the sample repeated if necessary.
if (x == getHi() || x == getLo()) {
return sample();
}
return x;
}
{code}
There is an unsolved issue in that there must actually be a value in between
the bounds. This will create a stack overflow:
{code:java}
ContinuousUniformSampler.of(rng, 3.25, 3.25, true).sample();
{code}
The factory constructor must verify that the lower and upper bound have a
double value in between them. I will add a fix for this when I have considered
how to solve the problem.
> "ContinuousUniformSampler" with both bounds excluded
> ----------------------------------------------------
>
> Key: RNG-145
> URL: https://issues.apache.org/jira/browse/RNG-145
> Project: Commons RNG
> Issue Type: New Feature
> Components: sampling
> Reporter: Gilles Sadowski
> Priority: Major
> Fix For: 1.4
>
>
> In class {{RandomUtils}}, Commons Math provides
> {code}
> public double nextUniform(double lo, double hi) { /* ... */ }
> {code}
> where both bounds are excluded.
> Should the feature be added to {{ContinuousUniformSampler}}?
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
