commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles Sadowski <gil...@harfang.homelinux.org>
Subject Re: [MATH] Test failure in Continuum
Date Mon, 06 Aug 2012 00:30:09 GMT
> >> [...]
> >> The original code above illustrates the convenience of being able to
> >> just make direct calls to randomData.nextXxx, which is why this
> >> class exists ;)
> > As I wrote in another post, I'm not against the convenience methods. But
> > IMO, they should be located in a new "DistributionUtils" class.
> 
> Why?  RandomData is pretty descriptive and exactly what these
> methods do.  They generate random data.

Fine...
But can we plan to merge "RandomData" and "RandomDataImpl"?

> > And we should also find a way to remove the code duplication (in the
> > distribution's "sample()" method and in the corresponding "next..." method).
> 
> +1 - the implementations can be moved.   When I last looked
> carefully at this, the distribution methods were delegating to impls
> in RandomDataImpl.  What we have agreed is to move the impls into
> the distribution classes for the basic sampling methods.  That
> should not be too hard to do.  I will do that if no one beats me to it.

I did it in r1363604.
What still needs to be done is redirect the "next..." to the "sample()"
method of the appropriate distribution.
But I had already raised the issue of efficiency: each call to e.g.
  nextInt(p, q)
will entail the instantiation of a UniformRealDistribution object.

What could be done is 
1. create a static method in the distribution class
2. have the "sample()" method call that one

---
public class UniformRealDistribution extends ... {
  // ...

  public static int nextInt(RandomGenerator rng,
                            int a,
                            int b) {
      final double u = rng.nextDouble();
      return u * b + (1 - u) * a;
  }

  public int sample() {
    // Here "random", "lower" and "upper" are instance variables.
    return nextInt(random, lower, upper);
  }
}
---

And "nextInt" from "RandomDataImpl" would also be redirected to the static
method in the distribution class:

---
import org.apache.commons.math3.distribution.UniformRealDistribution;

public class RandomDataImpl ... {
  // ...

  public int nextInt(int lower, int upper) {
    return UniformRealDistribution.nextInt(getRan(), lower, upper);
  }
}
---

OK?


Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message