[ 
https://issues.apache.org/jira/browse/MATH-310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12770687#action_12770687
 ] 

Mikkel Meyer Andersen commented on MATH-310:
--------------------------------------------

Yeah, it probably would be a good idea. Especially when it's implicitly assumed 
that 1 is not possible to get (because of the RandomGeneratorImpl), but it's 
really not a contract. Maybe include 1 in the test although it's not possible 
with traditional RNG. And also write that the nextDouble-method in the 
RandomNumber generator should provide a double between 0 and 1, either in- or 
exclusive.

> Supply nextSample for all distributions with inverse cdf using inverse 
> transform sampling approach
> --------------------------------------------------------------------------------------------------
>
>                 Key: MATH-310
>                 URL: https://issues.apache.org/jira/browse/MATH-310
>             Project: Commons Math
>          Issue Type: Improvement
>    Affects Versions: 2.0
>            Reporter: Mikkel Meyer Andersen
>            Priority: Minor
>         Attachments: patch_proposal
>
>   Original Estimate: 3h
>  Remaining Estimate: 3h
>
> To be able to generate samples from the supported probability distributions, 
> a generic function nextSample is implemented in 
> AbstractContinuousDistribution and AbstractIntegerDistribution. This also 
> gives the possibility to override the method if better algorithms are 
> available for specific distributions as shown in the small example with the 
> exponential distribution.
> Because the nextExponential is used several places: in nextPoisson it can be 
> replaces by an instance if the ExponentialDistribution and in ValueServer it 
> can as well, although maybe not in as natural maner as the other. 
> This problem with the Exponential is a special problem. In general the 
> nextSample-approaches immediately gives the possibility the sample from all 
> the distributions with inverse cdf instead just only a couple.
> Only AbstractContinuousDistribution and AbstractIntegerDistribution extends 
> AbstractDistribution, and both AbstractIntegerDistribution and 
> AbstractContinuousDistribution has an inverseCumulativeProbability-function. 
> But in AbstractContinuousDistribution the inverse cdf returns a double, and 
> at AbstractIntegerDistribution it - naturally - returns an integer. Therefor 
> the nextSample is not put on AbstractDistribution, but on each extension with 
> different return types.
> RandomGenerator as parameter instead of getting a RNG inside the nextSample, 
> because one typically wants to use the same RNG because often several random 
> samples are wanted. Another option is to have a RNG as a field in the class, 
> but that would be more ugly and also result in several RNGs at runtime.
> The nextPoisson etc. ought to be moved as well, if the enhancement is 
> accepted, but it should be a quick fix.
> Tests has to be written for this change as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to