-
Notifications
You must be signed in to change notification settings - Fork 22
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
Generate floats with Normal/Gaussian distribution #30
Conversation
We plan to add some documentation when this is merged! 😉 |
Very cool! 🎉 |
lib/rantly/generator.rb
Outdated
case distribution | ||
when :normal | ||
params[:center] ||= 0 | ||
params[:scale] || = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Syntax error here when I ran it. Remove extra space and align conditional assignment operator.
lib/rantly/generator.rb
Outdated
raise "The distribution scale should be greater than zero" unless params[:scale].positive? | ||
# Sum of 6 draws from a uniform distribution give as a draw of a normal | ||
# distribution centered in 3 | ||
([rand, rand, rand, rand, rand, rand].sum - 3) * params[:scale] + center |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Syntax error with center
. Probably mean params[:center]
?
A test would be helpful for this, but it can be added later. |
bfdf885
to
8e2712d
Compare
@abargnesi I think you need to enable Travis builds for |
@abargnesi please rereview it. I tried the code works now and I added a test. In the In the test a random center is generated. An array of 100 floats with a normal distribution centered in this center is also generated. The test checks that the average of the points is close to the center. I do not come up with a simpler way to test this. @vicgalle you may also want to take a look to the test 😉 |
Good point regarding travis-ci, thanks! I enabled it for rantly-rb/rantly, see here. |
lib/rantly/generator.rb
Outdated
raise "The distribution scale should be greater than zero" unless params[:scale].positive? | ||
# Sum of 6 draws from a uniform distribution give as a draw of a normal | ||
# distribution centered in 3 (central limit theorem). | ||
([rand, rand, rand, rand, rand, rand].sum - 3) * params[:scale] + params[:center] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enumerable#sum was added in ruby 2.4.0. Since ruby 2.3 is still being maintained (see here) we should widen support by using reduce
, e.g.:
[...].reduce(0, &:+)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually in Travis there are many Ruby versions: https://travis-ci.org/rantly-rb/rantly/builds/320015060
Si I guess we want to support all of them. I'll change this then. 😉
test/rantly_test.rb
Outdated
normal_points = Array.new(100){ float(:normal, { center: center }) } | ||
[center, normal_points] | ||
}.check{ |center, normal_points| | ||
average_center = normal_points.sum / 100 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same argument against using Enumerable#sum
.
Feel free to make additional commits to your branch for this PR. We can always squash all commits before merging to make |
@abargnesi please re-review it 😉 There are some Travis builds failing: Couldn't find a repository matching this job but it seems unrelated to the changes as the test are passing, when reporting the coverage the job is not found 😕 |
4857ea7
to
7089c9f
Compare
Test are passing now, I had used the @abargnesi @vicgalle should I merge this then? 😉 |
This allow us to generate points with more probability when we are closer to a limit/key point. The sum of 6 draws from a uniform distribution give as a draw of a normal distribution centered in 3. This is commonly used in programming for simplicity and it is proved by the central limit theorem. Contributes to rantly-rb#29 Co-authored-by: Víctor Gallego Alcalá <vicgalle@ucm.es>
When generating floats using the normal distribution, allow to specify the scale. This allow us to decide how close or far away should be the generated points from the limit/key point. Contributes to rantly-rb#29 Co-authored-by: Víctor Gallego Alcalá <vicgalle@ucm.es>
In the test a random center is generated. An array of 100 floats with a normal distribution centered in this center is also generated. The test checks that the average of the points is close to the center.
No complains means I can merge it 😝 Merging.... 🚢 |
Add generations of floats using normal distribution, also called Gaussian distribution. This allow us to generate points with more probability when we are closer to a limit/key point. We also allow allow to specify the scale, which allow us to decide how close or far away should the generated points be from the limit/key point. 🤓
The sum of 6 draws from a uniform distribution give as a draw of a normal distribution centered in 3. This is commonly used in programming for simplicity and it is proved by the central limit theorem.
Closes #29.
Not sure if we should add some kind of tests here. 🤔
Pair-programmed by @Ana06 and @vicgalle.