Skip to content

better handling of the random value#832

Merged
mitzimorris merged 1 commit intodevelopfrom
write_actual_random_value_ver_2
Feb 26, 2020
Merged

better handling of the random value#832
mitzimorris merged 1 commit intodevelopfrom
write_actual_random_value_ver_2

Conversation

@rok-cesnovar
Copy link
Copy Markdown
Member

Submisison Checklist

  • Run tests: ./runCmdStanTests.py src/test
  • Declare copyright holder and open-source license: see below

Summary:

Fixes #803 with a more general solution. This PR modifies the arg_seed class by adding _random_value variable.

In case of using the default value of -1, value() would still return -1, while random_value would return the time dependant generated pseudo-random number. This circumvents the issue that we want the seed to be an unsigned int while also allowing -1 as a valid value.

The previous solution wrote the psedu-random number to directly to value which was a signed integer and that led to issues.

The alternative is to only use the non-negative values and stay with signed integer. But I dont think that was the intention when refactoring pseudo random generation awhile back.

Copyright and Licensing

Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company): Rok Češnovar

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:

@stan-buildbot
Copy link
Copy Markdown
Contributor


Name Old Result New Result Ratio Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan 4.8 4.82 1.0 -0.43% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.02 0.02 0.94 -6.03% slower
eight_schools/eight_schools.stan 0.09 0.09 1.0 0.07% faster
gp_regr/gp_regr.stan 0.22 0.21 1.01 1.15% faster
irt_2pl/irt_2pl.stan 6.07 6.06 1.0 0.23% faster
performance.compilation 88.06 87.34 1.01 0.81% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 7.33 7.45 0.98 -1.6% slower
pkpd/one_comp_mm_elim_abs.stan 20.52 20.55 1.0 -0.12% slower
sir/sir.stan 88.95 96.66 0.92 -8.68% slower
gp_regr/gen_gp_data.stan 0.05 0.05 1.0 -0.4% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan 2.98 2.96 1.01 0.58% faster
pkpd/sim_one_comp_mm_elim_abs.stan 0.31 0.31 0.99 -1.34% slower
arK/arK.stan 1.75 1.73 1.01 0.67% faster
arma/arma.stan 0.76 0.76 1.0 -0.19% slower
garch/garch.stan 0.63 0.63 1.0 -0.36% slower
Mean result: 0.990312530962

Jenkins Console Log
Blue Ocean
Commit hash: 091ae9c


Machine information ProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010

CPU:
Intel(R) Xeon(R) CPU E5-1680 v2 @ 3.00GHz

G++:
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

Clang:
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

@stan-buildbot
Copy link
Copy Markdown
Contributor


Name Old Result New Result Ratio Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan 4.86 4.85 1.0 0.24% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.02 0.02 0.96 -4.0% slower
eight_schools/eight_schools.stan 0.09 0.09 0.98 -2.52% slower
gp_regr/gp_regr.stan 0.21 0.22 0.99 -0.89% slower
irt_2pl/irt_2pl.stan 6.06 6.1 0.99 -0.62% slower
performance.compilation 87.97 87.04 1.01 1.06% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 7.34 7.45 0.99 -1.48% slower
pkpd/one_comp_mm_elim_abs.stan 20.05 21.78 0.92 -8.63% slower
sir/sir.stan 90.32 88.9 1.02 1.57% faster
gp_regr/gen_gp_data.stan 0.04 0.05 0.96 -4.62% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan 2.94 2.95 1.0 -0.07% slower
pkpd/sim_one_comp_mm_elim_abs.stan 0.32 0.31 1.02 1.8% faster
arK/arK.stan 1.73 1.72 1.01 0.85% faster
arma/arma.stan 0.76 0.75 1.01 0.99% faster
garch/garch.stan 0.63 0.63 1.01 0.63% faster
Mean result: 0.990352462614

Jenkins Console Log
Blue Ocean
Commit hash: 091ae9c


Machine information ProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010

CPU:
Intel(R) Xeon(R) CPU E5-1680 v2 @ 3.00GHz

G++:
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

Clang:
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

Copy link
Copy Markdown
Member

@mitzimorris mitzimorris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good! many thanks

@mitzimorris mitzimorris merged commit 8256e3f into develop Feb 26, 2020
@rok-cesnovar rok-cesnovar deleted the write_actual_random_value_ver_2 branch March 4, 2020 06:24
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.

Random seed is not logged/written correctly

3 participants