Closed mitzimorris closed 1 month ago
@WardBrian - Ubuntu tests failing because of nodejs warning?
[run stan tests (ubuntu-20.04, 3.8)](https://github.com/stan-dev/stan/actions/runs/10059471447/job/27804774061#step:4:1737)
Process completed with exit code 136.
[run stan fvar-var model tests (ubuntu-20.04, 3.8)](https://github.com/stan-dev/stan/actions/runs/10059471447/job/27804773722)
The following actions uses Node.js version which is deprecated and will be forced to run on node20: actions/checkout@v3. For more info: https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/
run stan tests (ubuntu-20.04, 3.8)
The following actions uses Node.js version which is deprecated and will be forced to run on node20: actions/checkout@v3. For more info: https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/
the offending test uses the Stan CSV reader - but works on Mac. is there something cached that needs cleanup or????
The node warning is not the source of the error, its
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from ServicesLaplaceJacobian
[ RUN ] ServicesLaplaceJacobian.laplace_jacobian_adjust
Floating point exception (core dumped)
test/unit/services/optimize/laplace_jacobian_test --gtest_output="xml:test/unit/services/optimize/laplace_jacobian_test.xml" failed
exit now (07/23/24 13:21:14 UTC)
There is also a separate failure in Jenkins:
[ RUN ] StanIoStanCsvReader.skip_warmup
unknown file: Failure
C++ exception with description "Error with header of input file in parse" thrown in the test body.
[ FAILED ] StanIoStanCsvReader.skip_warmup (1 ms)
[ RUN ] StanIoStanCsvReader.missing_data
[ OK ] StanIoStanCsvReader.missing_data (10 ms)
[----------] 11 tests from StanIoStanCsvReader (69 ms total)
[----------] Global test environment tear-down
[==========] 11 tests from 1 test suite ran. (69 ms total)
[ PASSED ] 10 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] StanIoStanCsvReader.skip_warmup
1 FAILED TEST
test/unit/io/stan_csv_reader_test --gtest_output="xml:test/unit/io/stan_csv_reader_test.xml" failed
Looking at the laplace test, it is setting up a writer without a comment prefix
stan::callbacks::stream_writer sample_writer(sample1_ss, "");
and then later feeding that back into the csv_reader, which is now expecting to find #
s, maybe?
test/unit/io/stan_csv_reader_test --gtest_output="xml:test/unit/io/stan_csv_reader_test.xml" failed
I just fixed that one - checked in a test file.
Name | Old Result | New Result | Ratio | Performance change( 1 - new / old ) |
---|---|---|---|---|
arma/arma.stan | 0.35 | 0.33 | 1.03 | 3.01% faster |
low_dim_corr_gauss/low_dim_corr_gauss.stan | 0.01 | 0.01 | 1.01 | 1.1% faster |
gp_regr/gen_gp_data.stan | 0.02 | 0.02 | 1.06 | 6.09% faster |
gp_regr/gp_regr.stan | 0.1 | 0.1 | 0.98 | -1.67% slower |
sir/sir.stan | 72.55 | 68.8 | 1.05 | 5.17% faster |
irt_2pl/irt_2pl.stan | 4.44 | 3.96 | 1.12 | 10.85% faster |
eight_schools/eight_schools.stan | 0.06 | 0.06 | 1.06 | 6.07% faster |
pkpd/sim_one_comp_mm_elim_abs.stan | 0.26 | 0.24 | 1.11 | 10.09% faster |
pkpd/one_comp_mm_elim_abs.stan | 20.14 | 18.44 | 1.09 | 8.42% faster |
garch/garch.stan | 0.47 | 0.41 | 1.14 | 12.13% faster |
low_dim_gauss_mix/low_dim_gauss_mix.stan | 2.86 | 2.6 | 1.1 | 9.0% faster |
arK/arK.stan | 1.81 | 1.72 | 1.05 | 4.79% faster |
gp_pois_regr/gp_pois_regr.stan | 2.85 | 2.69 | 1.06 | 5.49% faster |
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan | 9.23 | 8.37 | 1.1 | 9.32% faster |
performance.compilation | 196.43 | 187.85 | 1.05 | 4.37% faster |
Mean result: 1.0686199247172619
Jenkins Console Log Blue Ocean Commit hash: c83a397be67c59ba135871db25c53717e78d796e
that upstream test failure is because now "fixed params" outputs aren't parsing. the hack would be to make sure that comment block at beginning of CSV file always contains a line "# algorithm = fixed_param". will file issue on CmdStan.
update: issue exists since 3 years: https://github.com/stan-dev/cmdstan/issues/1029
I am still not sure I follow what the goal of this PR is -- is there a bug it is fixing?
This will make the returned stan_csv object correctly capture the adaptation information in the case that the Stan CSV file has saved warmup draws - that's technically a bug, no?
It will also ensure that the stan_csv object's samples
array contains only post-warmup draws, which, given the logic in the parse
function, appears to be what is intended. Without this fix, when the StanCSV output has saved warmup draws, then the returned samples
array contains a bunch of draws that aren't used anywhere, which is a waste of memory.
It will also simplify the logic in the stan/mcmc/chains
- which I how I ended up going down this rabbit hole.
Ah, I missed how this would fix reading adaptation. That is nice to have.
How positive are we that nobody uses the warmup draws read in by this class? I noticed this class is imported at least once in RStan
How positive are we that nobody uses the warmup draws read in by this class? I noticed this class is imported at least once in RStan
Imported, but no longer used? All the diagnostics were rewritten circa 2019 when the improved Rhat paper came out.
I filed https://github.com/stan-dev/cmdstan/pull/1286 on CmdStan which would make deciding when to skip warmup more straightforward.
Submission Checklist
./runTests.py src/test/unit
make cpplint
Summary
Added logic to skip warmup draws, if any present, more checks on consistency between config and output.
Intended Effect
see #3301
How to Verify
Unit tests
Side Effects
N/A
Documentation
N/A
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): Columbia University
By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses: