Skip to content

Revert "Merge pull request #814 from bugfix/display_actual_random_seed"#831

Merged
rok-cesnovar merged 1 commit intodevelopfrom
revert_write_actual_random_seed
Feb 25, 2020
Merged

Revert "Merge pull request #814 from bugfix/display_actual_random_seed"#831
rok-cesnovar merged 1 commit intodevelopfrom
revert_write_actual_random_seed

Conversation

@rok-cesnovar
Copy link
Copy Markdown
Member

This reverts commit 662d6ef, reversing
changes made to 85dd764.

Submisison Checklist

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

Summary:

My solution done in #814 turned out to be invalid.

int_argument* random_arg = dynamic_cast<int_argument*>(parser.arg("random")->arg("seed"));
    unsigned int random_seed;
    if (random_arg->is_default()) {
      random_seed = (boost::posix_time::microsec_clock::universal_time() - boost::posix_time::ptime(boost::posix_time::min_date_time)).total_milliseconds();
      random_arg->set_value(random_seed);

The random seed is unsigned int, but the solution done in #814 set the value to the argument which is a int argument (because we want to support -1 as the default value and the value used to get random seeds). And a few hours ago the random_seed has gone outside the int value range, see https://jenkins.mc-stan.org/blue/organizations/jenkins/CmdStan/activity.

Luckily, the random seed used in sampling is still random, just the printed value in stdout and csv is is now again -1, hence I am reopening #803.

Set value ignored it because it tests if the value is valid:

    bool set_value(const T& value) {
      if (is_valid(value)) {
        _value = value;
        return true;
      }
      return false;
    }

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:

…andom_seed"

This reverts commit 662d6ef, reversing
changes made to 85dd764.
@rok-cesnovar
Copy link
Copy Markdown
Member Author

rok-cesnovar commented Feb 25, 2020

@mcol would you mind reviewing this? All upstreams tests are currently failing due to this issue.

Copy link
Copy Markdown
Member

@mcol mcol left a comment

Choose a reason for hiding this comment

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

Approving, as it's just a revert and indeed I've seen many failures in upstream cmdstan tetsts.

@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.79 4.81 1.0 -0.44% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.02 0.02 0.96 -3.84% slower
eight_schools/eight_schools.stan 0.09 0.09 0.99 -1.16% slower
gp_regr/gp_regr.stan 0.21 0.22 0.99 -1.29% slower
irt_2pl/irt_2pl.stan 6.04 6.04 1.0 0.04% faster
performance.compilation 90.13 86.87 1.04 3.61% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 7.35 7.33 1.0 0.24% faster
pkpd/one_comp_mm_elim_abs.stan 21.19 20.56 1.03 2.94% faster
sir/sir.stan 90.46 90.29 1.0 0.19% faster
gp_regr/gen_gp_data.stan 0.05 0.05 0.96 -3.98% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan 2.95 2.95 1.0 -0.0% slower
pkpd/sim_one_comp_mm_elim_abs.stan 0.31 0.31 1.0 -0.47% slower
arK/arK.stan 1.74 1.74 1.0 -0.26% slower
arma/arma.stan 0.71 0.78 0.92 -8.5% slower
garch/garch.stan 0.59 0.63 0.94 -6.11% slower
Mean result: 0.988357628117

Jenkins Console Log
Blue Ocean
Commit hash: 12faa9d


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

@rok-cesnovar rok-cesnovar merged commit 0be9a8a into develop Feb 25, 2020
@rok-cesnovar rok-cesnovar deleted the revert_write_actual_random_seed branch February 25, 2020 10:59
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.

3 participants