You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Mikkel Meyer Andersen (JIRA)" <ji...@apache.org> on 2009/10/27 14:28:59 UTC
[jira] Created: (MATH-310) Supply nextSample for all distributions
with inverse cdf using inverse transform sampling approach
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
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.
Re: [Math] Code snippet on a JIRA page
Posted by sebb <se...@gmail.com>.
On 08/03/2010, Gilles Sadowski <gi...@harfang.homelinux.org> wrote:
> Hi.
>
> > Phil Steitz commented on MATH-310:
> > ----------------------------------
>
> > [...]
>
> The code examples which I tried to write in the comments on the JIRA web
> page do not not look nice; sorry. Is there a way to indicate that some part
> should be formatted verbatim?
{code}
{code}
Click the ? help button on the rhs for more info.
> Thanks,
>
> Gilles
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] Code snippet on a JIRA page
Posted by Phil Steitz <ph...@gmail.com>.
Gilles Sadowski wrote:
> Hi.
>
>> Phil Steitz commented on MATH-310:
>> ----------------------------------
>
>> [...]
>
> The code examples which I tried to write in the comments on the JIRA web
> page do not not look nice; sorry. Is there a way to indicate that some part
> should be formatted verbatim?
>
> Thanks,
> Gilles
Enclose the code snippets in JIRA {code} tags (note open and close
are the same):
{code}
some java code
{code}
Phil
[Math] Code snippet on a JIRA page
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hi.
> Phil Steitz commented on MATH-310:
> ----------------------------------
> [...]
The code examples which I tried to write in the comments on the JIRA web
page do not not look nice; sorry. Is there a way to indicate that some part
should be formatted verbatim?
Thanks,
Gilles
[jira] Commented: (MATH-310) Supply nextSample for all
distributions with inverse cdf using inverse transform sampling approach
Posted by "Ted Dunning (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12770513#action_12770513 ]
Ted Dunning commented on MATH-310:
----------------------------------
Looks nice.
A comment on the motivation for the 0 test might be nice, but I was able to figure it out with a moment of reflection so maybe it isn't necessary.
> 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.
[jira] Commented: (MATH-310) Supply nextSample for all
distributions with inverse cdf using inverse transform sampling approach
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12842614#action_12842614 ]
Gilles commented on MATH-310:
-----------------------------
> Not being able to reseed the PRNG is a pretty serious limitation when generating random data.
As I said, for those who need rich features, they can use them with your preferred option, namely through directly instantiating an object of type "RandomData".
> Having to maintain and document random data generation in two places [...]
This just reduces to e.g.
---CUT---
public class PoissonDistributionImpl extends ... {
/**
* Provide basic sampling functionality. This method directly calls the
* the {@link RandomData#nextPoisson() nextPoisson} method in {@link RandomData}.
* For more control, please use that class directly.
* @see {@link RandomDataImpl}.
*/
long nextSample() { randomData.nextPoisson(); }
}
---CUT---
> Making the distribution classes [...] stateful [...]
This is really an "implementation detail" (as it is completely hidden), that is, if you accept that users cannot access the RNG machinery from this side.
> 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.
[jira] Issue Comment Edited: (MATH-310) Supply nextSample for all
distributions with inverse cdf using inverse transform sampling approach
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12842614#action_12842614 ]
Gilles edited comment on MATH-310 at 3/8/10 2:34 PM:
-----------------------------------------------------
> Not being able to reseed the PRNG is a pretty serious limitation when generating random data.
As I said, for those who need rich features, they can use them with your preferred option, namely through directly instantiating an object of type "RandomData".
> Having to maintain and document random data generation in two places [...]
This just reduces to e.g.
{code:title=PoissonDistributionImpl.java|borderStyle=solid}
public class PoissonDistributionImpl extends ... {
/**
* Provide basic sampling functionality. This method directly calls the
* the {@link RandomData#nextPoisson() nextPoisson} method in {@link RandomData}.
* For more control, please use that class directly.
* @see {@link RandomDataImpl}.
*/
long nextSample() { randomData.nextPoisson(); }
}
{code}
> Making the distribution classes [...] stateful [...]
This is really an "implementation detail" (as it is completely hidden), that is, if you accept that users cannot access the RNG machinery from this side.
was (Author: erans):
> Not being able to reseed the PRNG is a pretty serious limitation when generating random data.
As I said, for those who need rich features, they can use them with your preferred option, namely through directly instantiating an object of type "RandomData".
> Having to maintain and document random data generation in two places [...]
This just reduces to e.g.
---CUT---
public class PoissonDistributionImpl extends ... {
/**
* Provide basic sampling functionality. This method directly calls the
* the {@link RandomData#nextPoisson() nextPoisson} method in {@link RandomData}.
* For more control, please use that class directly.
* @see {@link RandomDataImpl}.
*/
long nextSample() { randomData.nextPoisson(); }
}
---CUT---
> Making the distribution classes [...] stateful [...]
This is really an "implementation detail" (as it is completely hidden), that is, if you accept that users cannot access the RNG machinery from this side.
> 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.
[jira] Commented: (MATH-310) Supply nextSample for all
distributions with inverse cdf using inverse transform sampling approach
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12842500#action_12842500 ]
Phil Steitz commented on MATH-310:
----------------------------------
See discussion here: http://markmail.org/message/kolivuytbt5cj25s
As stated on the mailing list, I am +0/1 on the idea of adding generic inversion-based generators that work with any invertible distribution; but I still do not see attaching them to the distribution implementations as a good idea. This is for three reasons: 0) I see it as poor separation of concerns (admittedly this is a matter of taste, but I do not see sourcing random deviates as an essential behavior of a probability distribution) 1) if the implementation is *only* inversion-based, it will be naive for some distributions and we do not want users to get a bad impl by default 2) to fix 1) we have to essentially refactor our package structure to place random data generation into the distributions package, causing users to have to instantiate distributions and also configure generators to get deviates. I see it as simpler and more natural to use a RandomData instance. I am -1 on dropping the random package. Therefore, I am not in favor of attaching this functionality to the distributions.
I propose that we resolve this issue by
1) Extend RandomDataImpl to include deviate generation from all currently implemented distributions, using generic inversion as defined below where this is the best available method, or where we have not yet implemented a better one. In 3.0, we can either eliminate the RandomData interface or extend it to include the new methods.
2) Add a generic nextInversionDeviate(distribution, generator) method similar to that defined in the patch to RandomDataImpl. Doc the fact that for distributions having a nextXxx implementation, this should be used in place of nextInversionDeviate unless inversion is specifically desired.
> 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.
[jira] Commented: (MATH-310) Supply nextSample for all
distributions with inverse cdf using inverse transform sampling approach
Posted by "Mikkel Meyer Andersen (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Resolved: (MATH-310) Supply nextSample for all distributions
with inverse cdf using inverse transform sampling approach
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-310?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phil Steitz resolved MATH-310.
------------------------------
Resolution: Fixed
Fix completed in r949895.
> 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
> Fix For: 2.2
>
> 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.
[jira] Updated: (MATH-310) Supply nextSample for all distributions
with inverse cdf using inverse transform sampling approach
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-310?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phil Steitz updated MATH-310:
-----------------------------
Fix Version/s: 2.2
> 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
> Fix For: 2.2
>
> 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.
[jira] Commented: (MATH-310) Supply nextSample for all
distributions with inverse cdf using inverse transform sampling approach
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12842510#action_12842510 ]
Gilles commented on MATH-310:
-----------------------------
Wouldn't something along the following lines satisfy everybody's concerns?
In each "...Distribution" interface, add a "nextSample()" (with the appropriate return type). In each "...DistributionImpl", add a "RandomData" instance variable to be used within the "nextSample()" implementation. E.g. in "PoissonDistributionImpl.java"
---CUT---
import org.apache.commons.math.random.RandomData;
import org.apache.commons.math.random.RandomDataImpl;
public class PoissonDistributionImpl extends ... {
private final RandomData randomData = new RandomDataImpl();
// ....
long nextSample() {
randomData.nextPoisson();
}
}
---CUT---
> 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.
[jira] Commented: (MATH-310) Supply nextSample for all
distributions with inverse cdf using inverse transform sampling approach
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12842514#action_12842514 ]
Gilles commented on MATH-310:
-----------------------------
>From what I've read in the ML thread, people that want to have "nextSample()" don't really care about configuring the RNG, so you don't have to expose it to provide that syntactic sugar.
Those who would like more flexibility (like configuring the RNG) should use "RandomData" explicitely.
And the concern that all implementations dealing with sampling stay in package "random" is fulfilled.
> 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.
[jira] Commented: (MATH-310) Supply nextSample for all
distributions with inverse cdf using inverse transform sampling approach
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12842511#action_12842511 ]
Phil Steitz commented on MATH-310:
----------------------------------
The problem is that to be really useful, this requires that you expose the RandomData instance and allow the RandomGenerator to be configured. This contributes to the aroma of bad separation of concerns, IMO. To make it a little clearer why I see this as bad separation of concerns, consider that you could accomplish the same thing by putting the impl in each of the distributions and then having RandomDataImpl use the impls in the distributions. That seems backwards to me.
> 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.
[jira] Issue Comment Edited: (MATH-310) Supply nextSample for all
distributions with inverse cdf using inverse transform sampling approach
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12842510#action_12842510 ]
Gilles edited comment on MATH-310 at 3/8/10 2:33 PM:
-----------------------------------------------------
Wouldn't something along the following lines satisfy everybody's concerns?
In each "...Distribution" interface, add a "nextSample()" (with the appropriate return type). In each "...DistributionImpl", add a "RandomData" instance variable to be used within the "nextSample()" implementation. E.g. in "PoissonDistributionImpl.java"
{code:title=PoissonDistributionImpl.java|borderStyle=solid}
import org.apache.commons.math.random.RandomData;
import org.apache.commons.math.random.RandomDataImpl;
public class PoissonDistributionImpl extends ... {
private final RandomData randomData = new RandomDataImpl();
// ....
long nextSample() {
randomData.nextPoisson();
}
}
{code}
was (Author: erans):
Wouldn't something along the following lines satisfy everybody's concerns?
In each "...Distribution" interface, add a "nextSample()" (with the appropriate return type). In each "...DistributionImpl", add a "RandomData" instance variable to be used within the "nextSample()" implementation. E.g. in "PoissonDistributionImpl.java"
---CUT---
import org.apache.commons.math.random.RandomData;
import org.apache.commons.math.random.RandomDataImpl;
public class PoissonDistributionImpl extends ... {
private final RandomData randomData = new RandomDataImpl();
// ....
long nextSample() {
randomData.nextPoisson();
}
}
---CUT---
> 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.
[jira] Commented: (MATH-310) Supply nextSample for all
distributions with inverse cdf using inverse transform sampling approach
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12842543#action_12842543 ]
Phil Steitz commented on MATH-310:
----------------------------------
Not being able to reseed the PRNG is a pretty serious limitation when generating random data. Having to maintain and document random data generation in two places is also not something that I personally look forward to. Making the distribution classes (in my opinion needlessly) stateful and forcing them all to initialize and drag along a RandomData instance is also not a good idea, IMO.
> 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.
[jira] Commented: (MATH-310) Supply nextSample for all
distributions with inverse cdf using inverse transform sampling approach
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12843204#action_12843204 ]
Phil Steitz commented on MATH-310:
----------------------------------
I guess I am OK with this, as long as RandomDataImpl retains its strategy of lazy initialization of SecureRandom and Random instances - so what is "dragged along" is not a big memory footprint or initialization overhead. This creates a dependency shared by all distributions, but if we are willing to manage it, then OK.
> 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.
[jira] Updated: (MATH-310) Supply nextSample for all distributions
with inverse cdf using inverse transform sampling approach
Posted by "Mikkel Meyer Andersen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-310?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mikkel Meyer Andersen updated MATH-310:
---------------------------------------
Attachment: patch_proposal
> 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.
[jira] Commented: (MATH-310) Supply nextSample for all
distributions with inverse cdf using inverse transform sampling approach
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12849199#action_12849199 ]
Gilles commented on MATH-310:
-----------------------------
Shall we implement this feature in 2.1?
As it is an addition, there isn't a high risk that it'll break existing code. [The only potential problem is if someone created his own implementation of one of the "...Distribution" interfaces. How likely is that?]
> 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
> Fix For: 2.2
>
> 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.