[jira] [Commented] (RNG-140) nextLong(long lo, long hi)
[
https://issues.apache.org/jira/browse/RNG-140?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17382236#comment-17382236
]
Alex Herbert commented on RNG-140:
--
I adapted this from the same sampler for int values. However this rejection
test was new. The DiscreteUniformSampler does not have a test for this.
Uniformity is only tested for the small range. When I merge the PR I'll update
the DisceteUniformSampler test to match.
> nextLong(long lo, long hi)
> --
>
> Key: RNG-140
> URL: https://issues.apache.org/jira/browse/RNG-140
> Project: Commons RNG
> Issue Type: Wish
> Components: sampling
>Reporter: Gilles Sadowski
>Priority: Minor
> Labels: api
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Replacement for functionality defined in
> [{{RandomUtils}}|https://gitbox.apache.org/repos/asf?p=commons-math.git;a=blob;f=commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/random/RandomUtils.java;h=60060e71d5bbe1d00878a1f54f8bb1ff88b65f11;hb=HEAD#l293].
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (RNG-140) nextLong(long lo, long hi)
[
https://issues.apache.org/jira/browse/RNG-140?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17382213#comment-17382213
]
Gilles Sadowski commented on RNG-140:
-
{quote}[...] if you want.
{quote}
No. ;)
You've just confirmed that the rejection test is sufficient.
2.12.0.0
> nextLong(long lo, long hi)
> --
>
> Key: RNG-140
> URL: https://issues.apache.org/jira/browse/RNG-140
> Project: Commons RNG
> Issue Type: Wish
> Components: sampling
>Reporter: Gilles Sadowski
>Priority: Minor
> Labels: api
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Replacement for functionality defined in
> [{{RandomUtils}}|https://gitbox.apache.org/repos/asf?p=commons-math.git;a=blob;f=commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/random/RandomUtils.java;h=60060e71d5bbe1d00878a1f54f8bb1ff88b65f11;hb=HEAD#l293].
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (RNG-140) nextLong(long lo, long hi)
[
https://issues.apache.org/jira/browse/RNG-140?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17382202#comment-17382202
]
Alex Herbert commented on RNG-140:
--
bq. IIUC, you were meaning that with this one, it is not necessary to have one
that checks that the same average number of samples fall in equal-size bins.
Given that we test the RNG using PractRand and BigCrush we know the long is
uniform across the entire range. So if you just exclude values outside the
range you are interested in, the remaining samples will be uniform inside the
range.
I can add a Chi-squared test using bins if you want. It just seemed overkill.
> nextLong(long lo, long hi)
> --
>
> Key: RNG-140
> URL: https://issues.apache.org/jira/browse/RNG-140
> Project: Commons RNG
> Issue Type: Wish
> Components: sampling
>Reporter: Gilles Sadowski
>Priority: Minor
> Labels: api
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Replacement for functionality defined in
> [{{RandomUtils}}|https://gitbox.apache.org/repos/asf?p=commons-math.git;a=blob;f=commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/random/RandomUtils.java;h=60060e71d5bbe1d00878a1f54f8bb1ff88b65f11;hb=HEAD#l293].
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (RNG-140) nextLong(long lo, long hi)
[
https://issues.apache.org/jira/browse/RNG-140?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17382119#comment-17382119
]
Gilles Sadowski commented on RNG-140:
-
bq. testSamplesWithLargeNonPowerOf2RangeIsRejectionMethod : Tests the sampler
uses nextLong and discards samples outside the range.
IIUC, you were meaning that with this one, it is not necessary to have one that
checks that the same average number of samples fall in equal-size bins.
> nextLong(long lo, long hi)
> --
>
> Key: RNG-140
> URL: https://issues.apache.org/jira/browse/RNG-140
> Project: Commons RNG
> Issue Type: Wish
> Components: sampling
>Reporter: Gilles Sadowski
>Priority: Minor
> Labels: api
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Replacement for functionality defined in
> [{{RandomUtils}}|https://gitbox.apache.org/repos/asf?p=commons-math.git;a=blob;f=commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/random/RandomUtils.java;h=60060e71d5bbe1d00878a1f54f8bb1ff88b65f11;hb=HEAD#l293].
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (RNG-140) nextLong(long lo, long hi)
[
https://issues.apache.org/jira/browse/RNG-140?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17382089#comment-17382089
]
Alex Herbert commented on RNG-140:
--
bq. Which is the test that does that?
testSamplesWithFullRange : Tests the sampler just returns the same as
nextLong().
testSamplesWithSmallNonPowerOf2Range : Tests the sampler returns the same as
nextLong(long) using an implementation of o.a.c.rng.core.BaseProvider. That
method is tested extensively as it is tested for all RNGs in the core package.
If it is the same output then it random and uniform.
testSamplesWithPowerOf2Range : Tests the sampler with zero bits or all bits
from the underlying RNG will output the lower and upper bound of the range.
testSamplesWithPowerOf2RangeIsBitShift : Tests the sampler uses the n most
significant bits from nextLong() using a bit shift.
testSamplesWithLargeNonPowerOf2RangeIsRejectionMethod : Tests the sampler uses
nextLong and discards samples outside the range.
If nextLong (or part of it for powers of 2) is uniform then the above tests
will create uniform output.
> nextLong(long lo, long hi)
> --
>
> Key: RNG-140
> URL: https://issues.apache.org/jira/browse/RNG-140
> Project: Commons RNG
> Issue Type: Wish
> Components: sampling
>Reporter: Gilles Sadowski
>Priority: Minor
> Labels: api
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Replacement for functionality defined in
> [{{RandomUtils}}|https://gitbox.apache.org/repos/asf?p=commons-math.git;a=blob;f=commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/random/RandomUtils.java;h=60060e71d5bbe1d00878a1f54f8bb1ff88b65f11;hb=HEAD#l293].
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (RNG-140) nextLong(long lo, long hi)
[
https://issues.apache.org/jira/browse/RNG-140?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17382072#comment-17382072
]
Gilles Sadowski commented on RNG-140:
-
bq. PR with the initial interfaces/classes
Thanks!
bq. [...] just testing the implementations directly against a reference RNG
that is known to be uniform.
Which is the test that does that?
> nextLong(long lo, long hi)
> --
>
> Key: RNG-140
> URL: https://issues.apache.org/jira/browse/RNG-140
> Project: Commons RNG
> Issue Type: Wish
> Components: sampling
>Reporter: Gilles Sadowski
>Priority: Minor
> Labels: api
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Replacement for functionality defined in
> [{{RandomUtils}}|https://gitbox.apache.org/repos/asf?p=commons-math.git;a=blob;f=commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/random/RandomUtils.java;h=60060e71d5bbe1d00878a1f54f8bb1ff88b65f11;hb=HEAD#l293].
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (RNG-140) nextLong(long lo, long hi)
[
https://issues.apache.org/jira/browse/RNG-140?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17382058#comment-17382058
]
Alex Herbert commented on RNG-140:
--
I've opened a PR with the initial interfaces/classes:
* LongSampler
* SharedStateLongSampler
* UniformLongSampler
The unit tests only have a single test class to test the sampler. These test
that the sampler uses either: UniformRandomProvider.nextLong(long) for a small
range; or UniformRandomProvider.nextLong() and rejects samples outside the
large range.
There did not seem to be a reason to duplicate the
DiscreteSamplerParametricTest for longs. That test asserts a sample is one of a
number of points (n), each sampled with a given probability. For example a
binomial distribution with n=10. For testing a large range of samples this is
not practical as an array cannot hold enough points for the range the sampler
can output.
An alternative is to duplicate the ContinuousSamplerParametricTest which
expects values in a range and the test requires the expected deciles of the
range. This could be adapted for the new sampler but since the sampler is
uniform the deciles are evenly spaced and the test is not much more informative
that just testing the implementations directly against a reference RNG that is
known to be uniform.
> nextLong(long lo, long hi)
> --
>
> Key: RNG-140
> URL: https://issues.apache.org/jira/browse/RNG-140
> Project: Commons RNG
> Issue Type: Wish
> Components: sampling
>Reporter: Gilles Sadowski
>Priority: Minor
> Labels: api
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Replacement for functionality defined in
> [{{RandomUtils}}|https://gitbox.apache.org/repos/asf?p=commons-math.git;a=blob;f=commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/random/RandomUtils.java;h=60060e71d5bbe1d00878a1f54f8bb1ff88b65f11;hb=HEAD#l293].
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (RNG-140) nextLong(long lo, long hi)
[
https://issues.apache.org/jira/browse/RNG-140?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17363024#comment-17363024
]
Gilles Sadowski commented on RNG-140:
-
bq. return an instance of the class from the factory constructor and not
interfaces?
I guess so because we don't know which interface the caller may need (as in the
case recently discussed).
> nextLong(long lo, long hi)
> --
>
> Key: RNG-140
> URL: https://issues.apache.org/jira/browse/RNG-140
> Project: Commons RNG
> Issue Type: Wish
> Components: sampling
>Reporter: Gilles Sadowski
>Priority: Minor
> Labels: api
>
> Replacement for functionality defined in
> [{{RandomUtils}}|https://gitbox.apache.org/repos/asf?p=commons-math.git;a=blob;f=commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/random/RandomUtils.java;h=60060e71d5bbe1d00878a1f54f8bb1ff88b65f11;hb=HEAD#l293].
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (RNG-140) nextLong(long lo, long hi)
[
https://issues.apache.org/jira/browse/RNG-140?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17363018#comment-17363018
]
Alex Herbert commented on RNG-140:
--
I can put the LongSampler and the implementation of UniformLongSampler into a
PR for review. It will be a lot of copy code as the tests will need expanding
to include the framework to test any LongSampler implementation.
Going forward shall we return an instance of the class from the factory
constructor and not interfaces? So any future change of interfaces that are
implemented will not be breaking compatibility.
> nextLong(long lo, long hi)
> --
>
> Key: RNG-140
> URL: https://issues.apache.org/jira/browse/RNG-140
> Project: Commons RNG
> Issue Type: Wish
> Components: sampling
>Reporter: Gilles Sadowski
>Priority: Minor
> Labels: api
>
> Replacement for functionality defined in
> [{{RandomUtils}}|https://gitbox.apache.org/repos/asf?p=commons-math.git;a=blob;f=commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/random/RandomUtils.java;h=60060e71d5bbe1d00878a1f54f8bb1ff88b65f11;hb=HEAD#l293].
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (RNG-140) nextLong(long lo, long hi)
[
https://issues.apache.org/jira/browse/RNG-140?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17363000#comment-17363000
]
Gilles Sadowski commented on RNG-140:
-
bq. I suggest [...] {{LongSampler}}
I'd have favoured {{DiscreteLongSampler}} (as alphabetical hint of functional
similarity) but you are probably right since it a low-level class required to
handle a different type. [The more so if the {{DiscreteSampler}} and
{{ContinuousSampler}} are going to disappear.]
bq. methods names are not as nice as the current {{sample}}.
{code}
interface ...
implements DoubleSupplier{
@Override
double getAsDouble();
default double sample() {
return getAsDouble();
}
}
{code}
> nextLong(long lo, long hi)
> --
>
> Key: RNG-140
> URL: https://issues.apache.org/jira/browse/RNG-140
> Project: Commons RNG
> Issue Type: Wish
> Components: sampling
>Reporter: Gilles Sadowski
>Priority: Minor
> Labels: api
>
> Replacement for functionality defined in
> [{{RandomUtils}}|https://gitbox.apache.org/repos/asf?p=commons-math.git;a=blob;f=commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/random/RandomUtils.java;h=60060e71d5bbe1d00878a1f54f8bb1ff88b65f11;hb=HEAD#l293].
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (RNG-140) nextLong(long lo, long hi)
[
https://issues.apache.org/jira/browse/RNG-140?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17362965#comment-17362965
]
Alex Herbert commented on RNG-140:
--
It could. But for the {{int}} case we have a sampler in the sampling package
that does the equivalent algorithm:
{code:java}
DiscreteSampler sampler = DiscreteUniformSampler.of(rng, lower, upper);
{code}
Pending an eventual update to Java 8 it would make more sense to have a sampler
interface that returns a long so we can move to support double, int or long
streams with the sampler as a supplier. Perhaps DiscreteLongSampler or just
LongSampler?
* ContinuousSampler (double)
* DiscreteSampler (int)
* DiscreteLongSampler (long)
* LongSampler (long)
Since the interfaces are functional interfaces (1 method) it would be cleaner
(in Java 8) to have the samplers just implement:
* DoubleSupplier.getAsDouble
* IntSupplier.getAsInt
* LongSupplier.getAsLong
The methods names are not as nice as the current {{sample}}. And we would have
to add the SharedStateSampler support to this:
* SharedStateDoubleSupplier
* SharedStateIntSupplier
* SharedStateLongSupplier
So maybe:
* LongSampler
* SharedStateLongSampler
Then add an additional method to the current DiscreteUniformSampler with long
instead of int arguments:
{code:java}
int lower = ...
int upper = ...
SharedStateDiscreteSampler sampler1 = DiscreteUniformSampler.of(rng, lower,
upper);
SharedStateLongSampler sampler2 = DiscreteUniformSampler.of(rng, (long) lower,
(long) upper);
{code}
Note that the factory method returns the interface type. It was mentioned that
a fluent API would return an instance of the class. If this is to be used in
the future (RNG 2.0) then the factory method should be in its own class. A
future API would be to replace ContinuousSampler and DiscreteSampler and with
the more specific DoubleSampler and IntSampler:
{code:java}
DoubleSampler s1 = UniformDoubleSampler.of(rng, 3.12, 4.23);
IntSampler s2 = UniformIntSampler.of(rng, 32, 45);
LongSampler s3 = UniformLongSampler.of(rng, 132L, 145L);
{code}
This cannot be done currently as factory methods are tied to returning
instances of the interface SharedState(Continuous|Discrete)Sampler. So those
interfaces must continue until a major release.
For now I suggest adding:
* LongSampler (interface)
* SharedStateLongSampler (interface)
* UniformLongSampler (sampler that implements the algorithm)
> nextLong(long lo, long hi)
> --
>
> Key: RNG-140
> URL: https://issues.apache.org/jira/browse/RNG-140
> Project: Commons RNG
> Issue Type: Wish
> Components: sampling
>Reporter: Gilles Sadowski
>Priority: Minor
> Labels: api
>
> Replacement for functionality defined in
> [{{RandomUtils}}|https://gitbox.apache.org/repos/asf?p=commons-math.git;a=blob;f=commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/random/RandomUtils.java;h=60060e71d5bbe1d00878a1f54f8bb1ff88b65f11;hb=HEAD#l293].
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (RNG-140) nextLong(long lo, long hi)
[
https://issues.apache.org/jira/browse/RNG-140?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17362947#comment-17362947
]
Gilles Sadowski commented on RNG-140:
-
Could the method be added to the {{UniformRandomProvider}} interface and
implemented in {{BaseProvider}}?
Code (from Commons Math):
{code:java}
public long nextLong(final long lower,
final long upper) {
if (lower >= upper) {
throw new
NumberIsTooLargeException(LocalizedFormats.LOWER_BOUND_NOT_BELOW_UPPER_BOUND,
lower, upper, false);
}
final long max = (upper - lower) + 1;
if (max <= 0) {
// Range is too wide to fit in a positive long (larger than
2^63);
// as it covers more than half the long range, we use directly a
// simple rejection method.
while (true) {
final long r = rng.nextLong();
if (r >= lower && r <= upper) {
return r;
}
}
} else if (max < Integer.MAX_VALUE){
// We can shift the range and generate directly a positive int.
return lower + rng.nextInt((int) max);
} else {
// We can shift the range and generate directly a positive long.
return lower + rng.nextLong(max);
}
}
{code}
> nextLong(long lo, long hi)
> --
>
> Key: RNG-140
> URL: https://issues.apache.org/jira/browse/RNG-140
> Project: Commons RNG
> Issue Type: Wish
> Components: sampling
>Reporter: Gilles Sadowski
>Priority: Minor
> Labels: api
>
> Replacement for functionality defined in
> [{{RandomUtils}}|https://gitbox.apache.org/repos/asf?p=commons-math.git;a=blob;f=commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/random/RandomUtils.java;h=60060e71d5bbe1d00878a1f54f8bb1ff88b65f11;hb=HEAD#l293].
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
