Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gaussian noise #224

Closed
arturoc opened this issue Aug 26, 2010 · 30 comments
Closed

gaussian noise #224

arturoc opened this issue Aug 26, 2010 · 30 comments
Labels
Milestone

Comments

@arturoc
Copy link
Member

@arturoc arturoc commented Aug 26, 2010

@bilderbuchi
Copy link
Member

@bilderbuchi bilderbuchi commented Dec 2, 2011

source here: https://github.com/andyr0id/ofxGaussian
It's just a couple lines, could be checked and added to ofMath, if needed

@ghost ghost assigned bilderbuchi Mar 26, 2012
@bilderbuchi
Copy link
Member

@bilderbuchi bilderbuchi commented Jun 7, 2012

I think I'll implement this, and maybe also the slightly better Ziggurat algorithm. But while I was browsing through the random number functions, I noticed some wonky things:
e.g. randNum = rand()/(RAND_MAX + 1.0); this will never give 1. Is it by design that we have functions which are 0 to 1(exclusive), and analogously for the other functions, when the sdtlib function is 0 to RAND_MAX (inclusive)?
Judging from git blame, mostly @ofTheo and @ofZach wrote that code afaict.

@bilderbuchi
Copy link
Member

@bilderbuchi bilderbuchi commented Jul 2, 2012

So @ofTheo and @ofZach, any input?

1 similar comment
@bilderbuchi
Copy link
Member

@bilderbuchi bilderbuchi commented Jul 16, 2012

So @ofTheo and @ofZach, any input?

@ofTheo
Copy link
Member

@ofTheo ofTheo commented Jul 16, 2012

@bilderbuchi so what you are saying is ofRandom(0, myVec.size()) will never = myVec.size() ?
and likewise ofRandom(0, 1) will never be 1.0 ?

@bilderbuchi if you want to do the implementation, that sounds good!

@bilderbuchi
Copy link
Member

@bilderbuchi bilderbuchi commented Jul 16, 2012

I haven't confirmed this in code yet, but yes, RAND_MAX/(RAND_MAX+1) can never yield 1. If this is not by design I can correct it but I wasn't sure if there is some hidden design reason behind this, which is why I asked.

@ofTheo
Copy link
Member

@ofTheo ofTheo commented Jul 16, 2012

I always wrote code assuming it could be 1 - but maybe @ofZach or @arturoc might know more?
this could be a separate issue as changing it could have some big implications.

but def go ahead on the gaussian noise!

@ofZach
Copy link
Contributor

@ofZach ofZach commented Jul 16, 2012

I think this may be a mistake, and worth looking at a fix for. (and also, looking at a few different implementations of random across platforms / environments just to be sure). I just took a look and alot of the random() implementations out there do seem to be written [0,1), ie from 0 and up to but not including 1. googling for "rand()/(RAND_MAX + 1.0)" is pretty interesting, and leads to alot of examples, which I guess this is based off of.

this is a really informative article:
http://www.azillionmonkeys.com/qed/random.html

at any rate, this seems wrong to me, and worth looking at a fix for.

@arturoc
Copy link
Member Author

@arturoc arturoc commented Jul 16, 2012

:) was going to post the same link

@ofZach
Copy link
Contributor

@ofZach ofZach commented Jul 16, 2012

actually, I'd have to dig back to older version of OF to check if this memory is true, but I think early on we had done random earlier on using %, and there was a link to this article in the code saying, let's fix this :) So this article is likely why we went with the RAND_MAX + 1....

@bilderbuchi
Copy link
Member

@bilderbuchi bilderbuchi commented Jul 17, 2012

ok, got enough to work with/look up for now, thanks guys. :-)

@kylemcdonald
Copy link
Contributor

@kylemcdonald kylemcdonald commented Dec 11, 2014

speaking of RNGs, i saw this was released recently http://www.pcg-random.org/

it has a feature-heavy c++ implementation, but there's also this version:

https://github.com/imneme/pcg-c-basic
https://github.com/imneme/pcg-c-basic/blob/master/pcg_basic.h
https://github.com/imneme/pcg-c-basic/blob/master/pcg_basic.c

which is supposed to be super fast, and a good RNG implementation (see comparison on home page).

@DomAmato
Copy link
Contributor

@DomAmato DomAmato commented Aug 10, 2015

How should we name this feature? ofGaussian with an overloaded function? or is this meant to replace the ofRandom functions?

@kylemcdonald
Copy link
Contributor

@kylemcdonald kylemcdonald commented Aug 10, 2015

i'm think the api would just be ofRandomGaussian(float mean = 0., float std = 1.) and it would not replace ofRandom.

@DomAmato
Copy link
Contributor

@DomAmato DomAmato commented Aug 11, 2015

Since there was discussion of moving to the light weight pcg random, they have a c++ implementation. its pretty simple, like 3 files

https://github.com/imneme/pcg-cpp

@DomAmato
Copy link
Contributor

@DomAmato DomAmato commented Aug 11, 2015

float ofRandomGaussian(float mean, float stddev) {
std::default_random_engine generator;
std::normal_distribution<float> distribution(mean, stddev);
return distribution(generator);
}

normal distributions were added in c++11 and I believe all of our platforms are now c++11 compliant?
http://www.cplusplus.com/reference/random/normal_distribution/

@kylemcdonald
Copy link
Contributor

@kylemcdonald kylemcdonald commented Aug 11, 2015

yes, i think that's correct. though i'd look at how the seed for ofRandom works, reinitializing it on every call might be a problem?

regardless, if you could write a short example to demonstrate behavior and submit a PR that'd be great.

@DomAmato
Copy link
Contributor

@DomAmato DomAmato commented Aug 12, 2015

I dont think that the seeding works because it deals with a different subset of code. std::normal_distribution requires the header to be included and an engine to be created. We could create a static initialization. I think we would need multiple functions though because in order to set the parameters of the distribution (if someone decides to change it like how it is posted above) you have to create a new parameter and pass it in.

like so:

static std::default_random_engine generator;
static std::normal_distribution<float> distribution(0, 1);

float ofRandomGaussian(float mean, float stddev) {
    std::normal_distribution<float>::param_type _param(mean, stddev);
    distribution.param(_param);
    return distribution(generator);
}

float ofRandomGaussian() {
    //this assumes the parameters dont change just get another number from the distribution
    return distribution(generator);
 }
@DomAmato
Copy link
Contributor

@DomAmato DomAmato commented Aug 12, 2015

also c++11 added in a lot more random features:

bernoulli_distribution
binomial_distribution
cauchy_distribution
chi_squared_distribution
discrete_distribution
exponential_distribution
extreme_value_distribution
fisher_f_distribution
gamma_distribution
geometric_distribution
lognormal_distribution
negative_binomial_distribution
normal_distribution
piecewise_constant_distribution
piecewise_linear_distribution
poisson_distribution
student_t_distribution
uniform_int_distribution
uniform_real_distribution
weibull_distribution

@kylemcdonald
Copy link
Contributor

@kylemcdonald kylemcdonald commented Aug 12, 2015

I see, in that case the original version makes sense I think :) my
intuition was just that creating an object might be "slow" and there could
be some faster way, but send a PR and maybe someone else has more insight :)

I don't think any of the other distributions are common enough for our
usual applications to include.

On Wednesday, August 12, 2015, Dominic Amato notifications@github.com
wrote:

also c++11 added in a lot more random features:

bernoulli_distribution
binomial_distribution
cauchy_distribution
chi_squared_distribution
discrete_distribution
exponential_distribution
extreme_value_distribution
fisher_f_distribution
gamma_distribution
geometric_distribution
lognormal_distribution
negative_binomial_distribution
normal_distribution
piecewise_constant_distribution
piecewise_linear_distribution
poisson_distribution
student_t_distribution
uniform_int_distribution
uniform_real_distribution
weibull_distribution


Reply to this email directly or view it on GitHub
#224 (comment)
.

@arturoc
Copy link
Member Author

@arturoc arturoc commented Aug 12, 2015

we should be using thread_local storage for this so this calls are thread safe. although thread_local statics are not yet well supported by clang and old versions of gcc so you'll need to use:

#if HAS_TLS
static thread_local std::default_random_engine generator;
static thread_local std::normal_distribution<float> distribution(0, 1);
#else
static std::default_random_engine generator;
static std::normal_distribution<float> distribution(0, 1);
#endif

not sure if using an anonymous namespace instead of static would allow to use thread_local under clang:

namespace{
thread_local std::default_random_engine generator;
thread_local std::normal_distribution<float> distribution(0, 1);
}

also perhaps we shouldn't be wrapping things like this that are available in the std and are not that difficult to use and instead just have examples on how to used them

@bilderbuchi
Copy link
Member

@bilderbuchi bilderbuchi commented Aug 12, 2015

also perhaps we shouldn't be wrapping things like this that are available in the std and are not that difficult to use and instead just have examples on how to used them

that also sounds like a sensible approach.

@DomAmato
Copy link
Contributor

@DomAmato DomAmato commented Aug 12, 2015

Still I wonder if we shouldn't port more of openframeworks over to C++11 standards and utilize the other random distribution engines. Pretty much all of ofRandom can be done using uniform_int_distribution or uniform_real_distribution since right now we are using rand() which is then put through a series of operations because of this http://www.azillionmonkeys.com/qed/random.html and in ofMath.h its even stated

\warning Many ofRandom-style functions wrap rand() which is not reentrant
/// or thread safe. To generate random numbers simultaneously in multiple
/// threads, consider using c++11 uniform_real_distribution.

maybe ofMath needs a facelift?

@ofTheo
Copy link
Member

@ofTheo ofTheo commented Aug 13, 2015

actually this was suggested recently.
see: #3890

also if you want to read the worlds longest issue discussing random :)
#3842

@DomAmato
Copy link
Contributor

@DomAmato DomAmato commented Aug 13, 2015

no kidding, I got worried when I looked to see how far I had made it into the thread and was only about 1/3rd of the way through.

Is there a preference to how its handled? I can test it on osx but obviously my main platform is windows. I will see how the thread local statics work and report back.

@DomAmato
Copy link
Contributor

@DomAmato DomAmato commented Aug 13, 2015

btw i've been dumping tests into a branch on my fork and will submit separate PR for each feature set but for those curious its here:
https://github.com/DomAmato/openFrameworks/tree/OFTinkering

@arturoc arturoc modified the milestones: 0.10.0, 0.9.1 Nov 9, 2015
@edap
Copy link
Contributor

@edap edap commented Dec 18, 2017

I was looking for a gaussian distribution function a while ago, and I ended up with:

// #include <random>
//  std::default_random_engine generator;
//  std::normal_distribution<float> distribution;


float ofxEnvelope::ofRandomGaussian(float mean, float stddev) {
    std::normal_distribution<float>::param_type _param(mean, stddev);
    distribution.param(_param);
    return distribution(generator);
}

But today I've find out that GLM has already everything we need, https://github.com/g-truc/glm/blob/0.9.6.3/glm/gtc/random.hpp, and since we are now using it, why not simply integrate the gaussian random distribution method from there? I think that also a sphericalRand and a diskRand methods would be pretty helpful. I can take care of the PR if we are agree.

Ping @arturoc @ofTheo

@bakercp
Copy link
Member

@bakercp bakercp commented Oct 11, 2019

@arturoc ...

@edap
Copy link
Contributor

@edap edap commented Oct 11, 2019

@ofTheo ofTheo closed this Oct 11, 2019
@edap
Copy link
Contributor

@edap edap commented Oct 11, 2019

Sorry I read the whole thread just now, on the phone I have seen just my last comment and I did not noticed all the previous discussion regarding this topic. Feel free to re-open and reconsider. My feeling is that what is in glm should stay in glm and should not be integrated in the core.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
You can’t perform that action at this time.