[
https://issues.apache.org/jira/browse/MATH-764?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13284641#comment-13284641
]
Dennis Hendriks edited comment on MATH-764 at 5/29/12 7:14 AM:
---------------------------------------------------------------
bq. [...] I would suggest, as Gilles mentions above, just using RandomDataImpl
directly, which provides direct support for sampling with configurable
RandomGenerator. The setup in the patch looks like a long way around the barn
to just get back to what is there already in RandomDataImpl.
In my opinion, RandomDataImpl should just provide random data, and should not
implement all the different distributions, as that is what the distributions
(classes) themselves are for. It seems we currently use a mixed approach: some
distributions are implemented as methods on the RandomDataImpl class
(nextPoisson, nextExponential, nextUniform, nextBeta, ...), while other
distributions (TriangularDistribution, ...) don't have a corresponding method
in the RandomDataImpl class, and exist solely as distribution class. The
methods in RandomDataImpl mostly seem to just use 'return
nextInversionDeviate(new SomeDistributionClass(param1, param2, ..., paramn));'
as method implementation. As such, we get a tight coupling between the
distributions and the RandomDataImpl class. Would it not be better to separate
the RandomData(Impl) from the distributions? Also, using nextInversionDeviate
is probably not what we want, as most distribution classes have a sample method
that provides a direct implementation instead of the generic
nextInversionDeviate...
was (Author: dhendriks):
bq [...] I would suggest, as Gilles mentions above, just using
RandomDataImpl directly, which provides direct support for sampling with
configurable RandomGenerator. The setup in the patch looks like a long way
around the barn to just get back to what is there already in RandomDataImpl.
In my opinion, RandomDataImpl should just provide random data, and should not
implement all the different distributions, as that is what the distributions
(classes) themselves are for. It seems we currently use a mixed approach: some
distributions are implemented as methods on the RandomDataImpl class
(nextPoisson, nextExponential, nextUniform, nextBeta, ...), while other
distributions (TriangularDistribution, ...) don't have a corresponding method
in the RandomDataImpl class, and exist solely as distribution class. The
methods in RandomDataImpl mostly seem to just use 'return
nextInversionDeviate(new SomeDistributionClass(param1, param2, ..., paramn));'
as method implementation. As such, we get a tight coupling between the
distributions and the RandomDataImpl class. Would it not be better to separate
the RandomData(Impl) from the distributions? Also, using nextInversionDeviate
is probably not what we want, as most distribution classes have a sample method
that provides a direct implementation instead of the generic
nextInversionDeviate...
> New sample() API should accept RandomGenerator as parameter
> -----------------------------------------------------------
>
> Key: MATH-764
> URL: https://issues.apache.org/jira/browse/MATH-764
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.0
> Reporter: Alex Bertram
> Attachments: sampler-refactor.diff
>
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> This may come to late as I know the 3.0 release is nearing completion, but I
> had some concerns about the new sample() method on the math3 RealDistribution
> interface.
> Specifically, there doesn't seem to be a way to supply a random generator to
> the sampler. Perhaps it would be better to have a factory method on the
> RealDistribution interface that accepted a RandomGenerator and returns an
> instance of some new interface, Sampler, which contains the sample() methods.
> That is:
> interface RealDistribution {
> Sampler createSampler(RandomGenerator generator);
> Sample createSampler(); // uses default RandomGenerator
> }
> interface Sampler {
> double sample();
> double[] sample(int sampleSize);
> }
--
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
