[jira] [Commented] (MATH-701) Seeding a default RNG
[
https://issues.apache.org/jira/browse/MATH-701?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13144170#comment-13144170
]
Phil Steitz commented on MATH-701:
--
I thought about changing the default seeding in AbstractWell and agree that
would be a good idea. I still like to supply the seed explicitly and document
it in RandomDataImpl, though, so users of that class know exactly what they are
getting by default. There is a little wrinkle here, too that keeping the
seeding expressed and documented in RandomDataImpl makes easier to keep track
of. If we ever implement hashcode in RandomDataImpl (or the Well generators),
things could get messed up if it does not separate generator instances the way
the system identity haschcode does.
> Seeding a default RNG
> -
>
> Key: MATH-701
> URL: https://issues.apache.org/jira/browse/MATH-701
> Project: Commons Math
> Issue Type: Bug
>Reporter: Gilles
>Assignee: Gilles
> Fix For: 3.0
>
>
> In "RandomDataImpl":
> {code}
> private RandomGenerator getRan() {
> if (rand == null) {
> rand = new JDKRandomGenerator();
> rand.setSeed(System.currentTimeMillis());
> }
> return rand;
> }
> {code}
> The conditional branch is used by "sample()" in
> "AbstractContinuousDistribution".
> When several "...Distribution" objects are instantiated in a short time
> interval, they are seeded with the same value.
--
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-701) Seeding a default RNG
[
https://issues.apache.org/jira/browse/MATH-701?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13144161#comment-13144161
]
Gilles commented on MATH-701:
-
Okay for the design discussion! ;)
But don't resolve just yet.
To avoid nasty surprises, I think that the "AbstractWell" (maybe others?)
should be modified so that default constructors are all seeded similarly.
"RandomDataImpl" as a "user" of the RNG should not have to worry how to best
seed the generator.
> Seeding a default RNG
> -
>
> Key: MATH-701
> URL: https://issues.apache.org/jira/browse/MATH-701
> Project: Commons Math
> Issue Type: Bug
>Reporter: Gilles
>Assignee: Gilles
> Fix For: 3.0
>
>
> In "RandomDataImpl":
> {code}
> private RandomGenerator getRan() {
> if (rand == null) {
> rand = new JDKRandomGenerator();
> rand.setSeed(System.currentTimeMillis());
> }
> return rand;
> }
> {code}
> The conditional branch is used by "sample()" in
> "AbstractContinuousDistribution".
> When several "...Distribution" objects are instantiated in a short time
> interval, they are seeded with the same value.
--
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-701) Seeding a default RNG
[
https://issues.apache.org/jira/browse/MATH-701?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13144122#comment-13144122
]
Phil Steitz commented on MATH-701:
--
I just committed (r1197626) a) change to Well generator as default for
non-secure generator b) seeding like the Harmony code. Can we resolve this and
take the refactoring discussion to the dev list?
> Seeding a default RNG
> -
>
> Key: MATH-701
> URL: https://issues.apache.org/jira/browse/MATH-701
> Project: Commons Math
> Issue Type: Bug
>Reporter: Gilles
>Assignee: Gilles
> Fix For: 3.0
>
>
> In "RandomDataImpl":
> {code}
> private RandomGenerator getRan() {
> if (rand == null) {
> rand = new JDKRandomGenerator();
> rand.setSeed(System.currentTimeMillis());
> }
> return rand;
> }
> {code}
> The conditional branch is used by "sample()" in
> "AbstractContinuousDistribution".
> When several "...Distribution" objects are instantiated in a short time
> interval, they are seeded with the same value.
--
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-701) Seeding a default RNG
[
https://issues.apache.org/jira/browse/MATH-701?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13143944#comment-13143944
]
Gilles commented on MATH-701:
-
* Harmony:
{code}
seed = System.currentTimeMillis() + hashCode();
{code}
* I'd also prefer setting both RNGs at construction.
A user would have to write
{code}
SecureRandom secRand = SecureRandom.getInstance(algorithm, provider);
{code}
in his application's code, and that would make it clearer for him why and how
an exception can be generated (and that CM's code is not the cause of it).
* I also favour a new "SecureRandomDataImpl" as a subclass of "RandomDataImpl";
the constructor would be something like:
{code}
public SecureRandomDataImpl(SecureRandom rng) {
super(rng);
}
{code}
This would ensure that _all_ the {{next...}} methods use a secure RNG, whereas
currently only some of the accessors have a secure alternative (e.g.
"nextGaussian" is not using "secRand"). This would be less confusing without
requiring long explanations...
> Seeding a default RNG
> -
>
> Key: MATH-701
> URL: https://issues.apache.org/jira/browse/MATH-701
> Project: Commons Math
> Issue Type: Bug
>Reporter: Gilles
>Assignee: Gilles
> Fix For: 3.0
>
>
> In "RandomDataImpl":
> {code}
> private RandomGenerator getRan() {
> if (rand == null) {
> rand = new JDKRandomGenerator();
> rand.setSeed(System.currentTimeMillis());
> }
> return rand;
> }
> {code}
> The conditional branch is used by "sample()" in
> "AbstractContinuousDistribution".
> When several "...Distribution" objects are instantiated in a short time
> interval, they are seeded with the same value.
--
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-701) Seeding a default RNG
[
https://issues.apache.org/jira/browse/MATH-701?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13143870#comment-13143870
]
Sebb commented on MATH-701:
---
If lazy init is dropped, then the rand field can be made final.
Can secRand be made final? This would mean dropping setSecureAlgorithm() in
favour of an extra ctor.
Since secRand is not always needed, this is an argument for making the secure
stuff a sub-class.
Alternatively, IODH could perhaps be used with secRand.
> Seeding a default RNG
> -
>
> Key: MATH-701
> URL: https://issues.apache.org/jira/browse/MATH-701
> Project: Commons Math
> Issue Type: Bug
>Reporter: Gilles
>Assignee: Gilles
> Fix For: 3.0
>
>
> In "RandomDataImpl":
> {code}
> private RandomGenerator getRan() {
> if (rand == null) {
> rand = new JDKRandomGenerator();
> rand.setSeed(System.currentTimeMillis());
> }
> return rand;
> }
> {code}
> The conditional branch is used by "sample()" in
> "AbstractContinuousDistribution".
> When several "...Distribution" objects are instantiated in a short time
> interval, they are seeded with the same value.
--
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-701) Seeding a default RNG
[
https://issues.apache.org/jira/browse/MATH-701?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13143706#comment-13143706
]
Phil Steitz commented on MATH-701:
--
+1 to changing the default in the RandomDataImpl to a Well generator
(documenting this)
+1 as well to trying to find a better default seed (documenting how it works).
Harmony still belongs to us, so we can certainly look at / imitate the impl
there.
The reason that the argumentless constructor exists is to allow users to accept
the default generator. The lazy initialization idiom was chosen because in some
cases, users may choose to invoke the setter instead of passing a generator as
a constructor argument. This is an older style no longer in use much in
[math]. I would be OK getting rid of this - i.e., having the argumentless
constructor take the hit to create a default instance.
-0 for separating out the secure stuff. RandomDataImpl is an aggregate class
that bundles a lot of commonly used random data generation methods. Just as
the random generator is pluggable, so is the secure random generator, which
makes the class convenient to use for applications that require both kinds of
random values. I think its simpler and more convenient to bundle things as
they are. Of course, I have been using this class for a long time, so I may
not be the best judge of what is simplest / easiest to use.
> Seeding a default RNG
> -
>
> Key: MATH-701
> URL: https://issues.apache.org/jira/browse/MATH-701
> Project: Commons Math
> Issue Type: Bug
>Reporter: Gilles
>Assignee: Gilles
> Fix For: 3.0
>
>
> In "RandomDataImpl":
> {code}
> private RandomGenerator getRan() {
> if (rand == null) {
> rand = new JDKRandomGenerator();
> rand.setSeed(System.currentTimeMillis());
> }
> return rand;
> }
> {code}
> The conditional branch is used by "sample()" in
> "AbstractContinuousDistribution".
> When several "...Distribution" objects are instantiated in a short time
> interval, they are seeded with the same value.
--
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-701) Seeding a default RNG
[
https://issues.apache.org/jira/browse/MATH-701?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13143559#comment-13143559
]
Gilles commented on MATH-701:
-
IIRC we discarded the solution of passing a RNG in the distribution's
constructor because the "sample" methods was construed as syntactic sugar in
order to access the "RandomData" functionality from within the
"...Distribution" classes.
It was also argued that people who would use this API (instead of directly
instantiating a "RandomDataImpl" object) would be satisfied with a default RNG,
whatever was deemed the best choice by the CM developers. Thus, if another
generator is indeed better, we should just plug it in instead of the current
"JDKRandomGenerator".
In the same line of argument, the possibility of seeding was also supposed to
be given up in exchange for a "sample" API at the distribution level.
In "AbstractContinuousDistribution", we have:
{code}
protected final RandomDataImpl randomData = new RandomDataImpl();
{code}
In effect, there is no reason not to replace that statement with:
{code}
protected final RandomDataImpl randomData = new RandomDataImpl(new
Well44497a());
{code}
As as side note: I don't understand why there is a default constructor in
"RandomDataImpl"; since all the methods need a RNG, it does not make much sense
to allow the object to be constructed without one!
Second side note: Why not separate the "secure" alternatives into another
implementation? There would be "RandomData" (with a "nextHexString" method) and
a "SecureRandomData" (also with a "nextHexString" method, instead of the
current "nextSecureHexString").
Back to this report's issue: The parent class "AbstractWell" seems to suffer
from the same problem because the default constructor calls
{{System.currentTimeMillis()}}.
We should have a look at how "java.util.Random" seeds its default instances so
that they can claim (excerpt form Javadoc):
{noformat}
This constructor sets the seed of the random number generator to a value very
likely to be
distinct from any other invocation of this constructor.
{noformat}
> Seeding a default RNG
> -
>
> Key: MATH-701
> URL: https://issues.apache.org/jira/browse/MATH-701
> Project: Commons Math
> Issue Type: Bug
>Reporter: Gilles
>Assignee: Gilles
> Fix For: 3.0
>
>
> In "RandomDataImpl":
> {code}
> private RandomGenerator getRan() {
> if (rand == null) {
> rand = new JDKRandomGenerator();
> rand.setSeed(System.currentTimeMillis());
> }
> return rand;
> }
> {code}
> The conditional branch is used by "sample()" in
> "AbstractContinuousDistribution".
> When several "...Distribution" objects are instantiated in a short time
> interval, they are seeded with the same value.
--
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-701) Seeding a default RNG
[
https://issues.apache.org/jira/browse/MATH-701?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13143322#comment-13143322
]
Phil Steitz commented on MATH-701:
--
I agree that there is a problem here; but I would solve it in
AbstractContinuousDistribution by passing a seeded generator to the
RandomDataImpl constructor. The code above is for the default case where no
generator has been supplied or set. For sampling, a Well generator would be a
better choice.
> Seeding a default RNG
> -
>
> Key: MATH-701
> URL: https://issues.apache.org/jira/browse/MATH-701
> Project: Commons Math
> Issue Type: Bug
>Reporter: Gilles
>Assignee: Gilles
> Fix For: 3.0
>
>
> In "RandomDataImpl":
> {code}
> private RandomGenerator getRan() {
> if (rand == null) {
> rand = new JDKRandomGenerator();
> rand.setSeed(System.currentTimeMillis());
> }
> return rand;
> }
> {code}
> The conditional branch is used by "sample()" in
> "AbstractContinuousDistribution".
> When several "...Distribution" objects are instantiated in a short time
> interval, they are seeded with the same value.
--
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-701) Seeding a default RNG
[
https://issues.apache.org/jira/browse/MATH-701?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13143300#comment-13143300
]
Gilles commented on MATH-701:
-
No, my point was that the seeding should just be removed: When several
distribution objects are instantiated, we usually don't want the samples to be
the same...
> Seeding a default RNG
> -
>
> Key: MATH-701
> URL: https://issues.apache.org/jira/browse/MATH-701
> Project: Commons Math
> Issue Type: Bug
>Reporter: Gilles
>Assignee: Gilles
> Fix For: 3.0
>
>
> In "RandomDataImpl":
> {code}
> private RandomGenerator getRan() {
> if (rand == null) {
> rand = new JDKRandomGenerator();
> rand.setSeed(System.currentTimeMillis());
> }
> return rand;
> }
> {code}
> The conditional branch is used by "sample()" in
> "AbstractContinuousDistribution".
> When several "...Distribution" objects are instantiated in a short time
> interval, they are seeded with the same value.
--
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-701) Seeding a default RNG
[
https://issues.apache.org/jira/browse/MATH-701?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13143287#comment-13143287
]
Mikkel Meyer Andersen commented on MATH-701:
I see your point, but at the same time it might be difficult to deal with
correctly. One work-around is to use the serialVersionUID field together with
System.currentTimeMillis(), but I am not sure if that is a good idea nor how
well it can be implemented.
> Seeding a default RNG
> -
>
> Key: MATH-701
> URL: https://issues.apache.org/jira/browse/MATH-701
> Project: Commons Math
> Issue Type: Bug
>Reporter: Gilles
>Assignee: Gilles
> Fix For: 3.0
>
>
> In "RandomDataImpl":
> {code}
> private RandomGenerator getRan() {
> if (rand == null) {
> rand = new JDKRandomGenerator();
> rand.setSeed(System.currentTimeMillis());
> }
> return rand;
> }
> {code}
> The conditional branch is used by "sample()" in
> "AbstractContinuousDistribution".
> When several "...Distribution" objects are instantiated in a short time
> interval, they are seeded with the same value.
--
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
