Feature/929 stansummary cli11 #965

mitzimorris · 2020-12-30T23:04:22Z

Submisison Checklist

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

Summary:

Replaced use of boost::program_options with lib CLI11,
which is a header-only C++11 library with appropriate open source license.

Intended Effect:

Pay down technical debt:

Simplify build process. Library boost::program_options is a non-header only library,
it made the build process longer, greatly increased the amount of the console output,
Simplify makefiles

How to Verify:

Run unit tests

Side Effects:

NA

Documentation:

NA

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:

Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

…4.1 (tags/RELEASE_600/final)

…ature/929-stansummary-cli11

stan-buildbot · 2020-12-30T23:23:34Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.45	3.38	1.02	2.0% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.92	-8.78% slower
eight_schools/eight_schools.stan	0.11	0.11	1.0	-0.16% slower
gp_regr/gp_regr.stan	0.15	0.15	0.98	-1.67% slower
irt_2pl/irt_2pl.stan	5.14	5.13	1.0	0.28% faster
performance.compilation	90.85	88.55	1.03	2.53% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.65	8.63	1.0	0.15% faster
pkpd/one_comp_mm_elim_abs.stan	29.16	28.95	1.01	0.72% faster
sir/sir.stan	136.31	139.62	0.98	-2.43% slower
gp_regr/gen_gp_data.stan	0.04	0.04	1.0	0.47% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.05	3.05	1.0	-0.02% slower
pkpd/sim_one_comp_mm_elim_abs.stan	0.37	0.38	0.99	-1.35% slower
arK/arK.stan	2.56	2.56	1.0	0.3% faster
arma/arma.stan	0.61	0.6	1.02	1.7% faster
garch/garch.stan	0.68	0.68	1.0	-0.42% slower
Mean result: 0.996194310003

Jenkins Console Log
Blue Ocean
Commit hash: 48ec304

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

bbbales2

The only changes here are the library, right? So it's the same features as the existing stansummary, just different code?

Edit: Can look at this tomorrow

src/test/interface/stansummary_test.cpp

mitzimorris · 2020-12-31T02:47:31Z

The only changes here are the library, right? So it's the same features as the existing stansummary, just different code?

correct - the CLI11 library is a better version of boost::program_options. updated the code to use the library; changed the unit tests to match messages generated by CLI11.

bbbales2

Review!

src/cmdstan/stansummary.cpp

bbbales2 · 2020-12-31T21:07:10Z

src/cmdstan/stansummary.cpp

+                 true);
+  app.add_option("input_files", filenames, "Sampler csv files.", true)
+      ->required()
+      ->check(CLI::ExistingFile);


This checks the input files are readable?

checks that they exist.
need to add checks for readibility, similar to check for writable output csv file.

Totally optional, but when I get the error for readability I don't get the friendly --help prompt:

bbales2@tadpole:~/cmdstan-develop$ bin/stansummary output4.csv Cannot read input csv file: output4.csv.

Compare this to:

bbales2@tadpole:~/cmdstan-develop$ bin/stansummary output12321.csv input_files: File does not exist: output12321.csv Run with --help for more information.

Also I don't get the --help prompt with the percentiles option:

bbales2@tadpole:~/cmdstan-develop$ bin/stansummary -p -5 output4.csv Option --percentiles -5: values must be in range (1,99), inclusive, and strictly increasing.

Very optional to add this.

can't figure out how to plug in custom help message via CLI11 - spent several hours looking.
will give it another go.

Eh if it's not easy let's not worry about it.

doable but not easy. worth doing as part of next PR.

src/test/utility.hpp

src/cmdstan/stansummary_helper.hpp

src/test/interface/stansummary_test.cpp

src/cmdstan/stansummary_helper.hpp

…4.1 (tags/RELEASE_600/final)

stan-buildbot · 2021-01-01T18:20:07Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.4	3.41	1.0	-0.15% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.93	-7.52% slower
eight_schools/eight_schools.stan	0.11	0.11	0.95	-4.87% slower
gp_regr/gp_regr.stan	0.15	0.16	0.95	-5.76% slower
irt_2pl/irt_2pl.stan	5.17	5.18	1.0	-0.27% slower
performance.compilation	91.45	89.76	1.02	1.85% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.61	8.63	1.0	-0.24% slower
pkpd/one_comp_mm_elim_abs.stan	29.64	30.52	0.97	-2.96% slower
sir/sir.stan	135.34	135.18	1.0	0.12% faster
gp_regr/gen_gp_data.stan	0.04	0.04	1.0	0.28% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.06	3.06	1.0	0.12% faster
pkpd/sim_one_comp_mm_elim_abs.stan	0.37	0.37	0.99	-0.52% slower
arK/arK.stan	2.55	2.57	0.99	-1.11% slower
arma/arma.stan	0.6	0.6	1.0	-0.04% slower
garch/garch.stan	0.69	0.67	1.03	3.01% faster
Mean result: 0.988826866668

Jenkins Console Log
Blue Ocean
Commit hash: 5001b23

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

…an-dev/cmdstan into feature/929-stansummary-cli11

mitzimorris · 2021-01-01T22:12:34Z

hi Ben, thanks for the thorough arg checking - this code should be far more robust.

also extremely useful w/r/t this PR: #925
there's a lot more checks that could/should be done for that PR, but it would be cool to get it in -
not sure I'll be able to get to it for this release.

mitzimorris · 2021-01-01T22:13:12Z

@bbbales2 - ready for re-review

…4.1 (tags/RELEASE_600/final)

stan-buildbot · 2021-01-01T22:34:05Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.45	3.36	1.03	2.63% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.99	-1.39% slower
eight_schools/eight_schools.stan	0.11	0.11	0.97	-2.93% slower
gp_regr/gp_regr.stan	0.16	0.15	1.05	4.82% faster
irt_2pl/irt_2pl.stan	5.13	5.19	0.99	-1.14% slower
performance.compilation	92.37	89.65	1.03	2.95% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.63	8.62	1.0	0.06% faster
pkpd/one_comp_mm_elim_abs.stan	31.11	29.22	1.06	6.09% faster
sir/sir.stan	137.26	132.37	1.04	3.56% faster
gp_regr/gen_gp_data.stan	0.04	0.04	1.02	2.41% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.04	3.07	0.99	-1.0% slower
pkpd/sim_one_comp_mm_elim_abs.stan	0.4	0.39	1.02	1.48% faster
arK/arK.stan	2.57	2.55	1.01	0.99% faster
arma/arma.stan	0.6	0.6	1.0	-0.33% slower
garch/garch.stan	0.67	0.68	0.99	-1.31% slower
Mean result: 1.01203272575

Jenkins Console Log
Blue Ocean
Commit hash: cd734b5

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

bbbales2

Looks good to me. Merge if you're happy with it. I don't think it's worth fussing over the --help thing if you've already spent time trying to get that working.

mitzimorris added 7 commits December 20, 2020 16:54

using CLI11 instead of boost program options; need to update unit tests

cde0b9e

using CLI11 instead of boost program options; need to update unit tests

9c89efa

works with example bernoulli

c44855c

update command runner to capture stderr

26986b3

updated unit tests; all tests passing

487b022

use CLI11 lib

bc3ef2f

updated unit tests; all tests passing

ce61fdb

mitzimorris requested review from bbbales2, rok-cesnovar and syclik December 30, 2020 23:05

mitzimorris added the makefile label Dec 30, 2020

yashikno and others added 3 commits December 30, 2020 23:06

Merge commit '817ccee5d9782d9afe14be7f8677b7da317cba19' into HEAD

d2e76ad

[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…

48ec304

…4.1 (tags/RELEASE_600/final)

Merge branch 'develop' of https://github.com/stan-dev/cmdstan into fe…

414964b

…ature/929-stansummary-cli11

bbbales2 reviewed Dec 31, 2020

View reviewed changes

src/test/interface/stansummary_test.cpp Show resolved Hide resolved

bbbales2 requested changes Dec 31, 2020

View reviewed changes

mitzimorris and others added 4 commits January 1, 2021 12:54

changes per code review

e9ca830

merge fix

51c4f8b

Merge commit 'd0d32a76954a37997e31e98746caa85d13fa2b3e' into HEAD

46019b7

[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…

5001b23

…4.1 (tags/RELEASE_600/final)

mitzimorris added 2 commits January 1, 2021 17:08

changes per code review

d6f3922

Merge branch 'feature/929-stansummary-cli11' of https://github.com/st…

e7de646

…an-dev/cmdstan into feature/929-stansummary-cli11

yashikno and others added 2 commits January 1, 2021 22:17

Merge commit 'd84693c228951e70483cc1a1da6f3da2e585c478' into HEAD

13d9657

[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…

cd734b5

…4.1 (tags/RELEASE_600/final)

bbbales2 approved these changes Jan 1, 2021

View reviewed changes

mitzimorris merged commit 66554a4 into develop Jan 2, 2021

WardBrian deleted the feature/929-stansummary-cli11 branch September 6, 2023 20:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/929 stansummary cli11 #965

Feature/929 stansummary cli11 #965

mitzimorris commented Dec 30, 2020

stan-buildbot commented Dec 30, 2020

bbbales2 left a comment •

edited

Loading

mitzimorris commented Dec 31, 2020

bbbales2 left a comment

bbbales2 Dec 31, 2020

mitzimorris Jan 1, 2021

mitzimorris Jan 1, 2021

bbbales2 Jan 1, 2021

bbbales2 Jan 1, 2021

mitzimorris Jan 1, 2021

bbbales2 Jan 1, 2021

mitzimorris Jan 2, 2021

stan-buildbot commented Jan 1, 2021

mitzimorris commented Jan 1, 2021

mitzimorris commented Jan 1, 2021

stan-buildbot commented Jan 1, 2021

bbbales2 left a comment

Feature/929 stansummary cli11 #965

Feature/929 stansummary cli11 #965

Conversation

mitzimorris commented Dec 30, 2020

Submisison Checklist

Summary:

Intended Effect:

How to Verify:

Side Effects:

Documentation:

Copyright and Licensing

stan-buildbot commented Dec 30, 2020

bbbales2 left a comment • edited Loading

Choose a reason for hiding this comment

mitzimorris commented Dec 31, 2020

bbbales2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stan-buildbot commented Jan 1, 2021

mitzimorris commented Jan 1, 2021

mitzimorris commented Jan 1, 2021

stan-buildbot commented Jan 1, 2021

bbbales2 left a comment

Choose a reason for hiding this comment

bbbales2 left a comment •

edited

Loading