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

Avoid calling log(0) when generating gaussian random variables #662

Merged
merged 3 commits into from
Mar 25, 2024

Conversation

edwintorok
Copy link
Contributor

Draft pull request because the following unit test fails (but it also fails before any of my changes):

File "test/unit_stats_rvs.ml", line 757, character 2:
FAIL Avg ppf error in 'gamma_shape_0.900000_scale_1.000000' <1E-28

   Expected: `true'
   Received: `false'

Raised at Alcotest_engine__Test.check in file "src/alcotest-engine/test.ml", line 200, characters 4-261
Called from Dune__exe__Unit_stats_rvs.test_rough_cdf in file "test/unit_stats_rvs.ml", line 757, characters 2-107
Called from Stdlib__List.iter in file "list.ml", line 110, characters 12-15
Called from Dune__exe__Unit_stats_rvs.test_rough_cdf_matches in file "test/unit_stats_rvs.ml", line 779, characters 2-45
Called from Alcotest_engine__Core.Make.protect_test.(fun) in file "src/alcotest-engine/core.ml", line 181, characters 17-23
Called from Alcotest_engine__Monad.Identity.catch in file "src/alcotest-engine/monad.ml", line 24, characters 31-35

sfmt_f64_1 is documented to include 0, which would result in `log(0) =
neg_infinity`.
use sfmt_f64_3 instead which is documented to return `(0, 1)` instead.

This should also match the paper, which says "UNI floats it to (0,1)":
> Marsaglia, George, and Wai Wan Tsang.
> "The ziggurat method for generating random variables."
> Journal of statistical software 5 (2000): 1-7.

See owlbarn#661

Signed-off-by: Edwin Török <edwin@etorok.net>
…at in (0,1)

Signed-off-by: Edwin Török <edwin@etorok.net>
…nfinity on Random.float returning 0

owlbarn#661

Signed-off-by: Edwin Török <edwin@etorok.net>
@jzstark jzstark marked this pull request as ready for review March 25, 2024 16:31
@jzstark jzstark merged commit 621cbff into owlbarn:main Mar 25, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants