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

Print errors to stderr instead of stdout; don't nag for fixed_param flag, just do it #992

Merged
merged 11 commits into from
Mar 12, 2021

Conversation

mitzimorris
Copy link
Member

Submisison Checklist

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

Summary:

  • command.hpp and main.cpp send error messages to cerr instead of cout
  • when method sample is specified, if model doesn't contain any parameters, run algorithm fixed_param and print message to console. detect if model has no parameters and run fixed param sampler automatically #953
  • slightly modified return code handling: code USAGE used only for unrecognized command line args. printing usage message returns code is OK.

Intended Effect:

make it easy for CmdStanX wrappers to report error messages via contents of stderr instead of making them scrape stdout.

How to Verify:

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:

@mitzimorris
Copy link
Member Author

we'll need to advertise this change because folks might have scripts or packages that scrape stderr and/or stdout.

I don't quite understand why services layer uses return codes plus callback writers; effectively, there are only two return codes: OK, not OK. plus whatever messages are written to stderr.

@stan-buildbot
Copy link
Contributor


Name Old Result New Result Ratio Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan 3.43 3.46 0.99 -0.77% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.02 0.02 0.96 -3.75% slower
eight_schools/eight_schools.stan 0.11 0.11 1.02 2.33% faster
gp_regr/gp_regr.stan 0.16 0.16 1.01 0.84% faster
irt_2pl/irt_2pl.stan 5.32 5.26 1.01 1.13% faster
performance.compilation 91.16 88.94 1.02 2.43% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 8.99 8.9 1.01 1.01% faster
pkpd/one_comp_mm_elim_abs.stan 30.39 30.24 1.0 0.47% faster
sir/sir.stan 131.89 127.88 1.03 3.04% faster
gp_regr/gen_gp_data.stan 0.04 0.04 1.21 17.22% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan 3.12 3.12 1.0 0.01% faster
pkpd/sim_one_comp_mm_elim_abs.stan 0.39 0.38 1.03 3.06% faster
arK/arK.stan 1.9 1.89 1.01 0.53% faster
arma/arma.stan 0.65 0.78 0.84 -19.64% slower
garch/garch.stan 0.5 0.56 0.89 -12.37% slower
Mean result: 1.00279471067

Jenkins Console Log
Blue Ocean
Commit hash: d537150


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
Member

@bbbales2 bbbales2 left a comment

Choose a reason for hiding this comment

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

Question about the return code on printing error message. Change of behavior for zero parameter model makes sense.

return err_code;
}
if (parser.help_printed())
return err_code;
return stan::services::error_codes::OK;
Copy link
Member

Choose a reason for hiding this comment

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

It looks like rm sets an error code here, so maybe we should too?

$ rm
rm: missing operand
Try 'rm --help' for more information.
$ echo $?
1

Copy link
Member Author

@mitzimorris mitzimorris Mar 11, 2021

Choose a reason for hiding this comment

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

if you only supply 1 arg, by default we print help.
if you request help, we print help.
checked make - when in a directory which has a Makefile present,
if runs whatever the default Makefile rule is.
when in a directory without a Makefile, return code 2
when invoked with -h, prints help, return code 0.

Copy link
Member Author

Choose a reason for hiding this comment

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

lines 136-137 are correct. changed line 128 - return code USAGE set when no args are given - this will result in help message being printed; and then program will exit. so if you get to line 136, user requested help to be printed, help was printed, all good. cf above example

src/cmdstan/arguments/argument_parser.hpp Outdated Show resolved Hide resolved
@mitzimorris
Copy link
Member Author

@bbbales2 - thanks for the code review - made changes, ready for re-review.

@stan-buildbot
Copy link
Contributor


Name Old Result New Result Ratio Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan 3.42 3.43 1.0 -0.36% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.02 0.02 1.0 -0.23% slower
eight_schools/eight_schools.stan 0.11 0.11 1.02 1.64% faster
gp_regr/gp_regr.stan 0.16 0.16 1.0 -0.22% slower
irt_2pl/irt_2pl.stan 5.31 5.33 1.0 -0.41% slower
performance.compilation 91.18 88.74 1.03 2.67% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 8.92 9.02 0.99 -1.16% slower
pkpd/one_comp_mm_elim_abs.stan 30.49 31.72 0.96 -4.04% slower
sir/sir.stan 128.75 128.01 1.01 0.57% faster
gp_regr/gen_gp_data.stan 0.04 0.03 1.28 21.8% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan 3.07 3.06 1.0 0.34% faster
pkpd/sim_one_comp_mm_elim_abs.stan 0.39 0.41 0.93 -7.31% slower
arK/arK.stan 1.92 2.58 0.74 -34.38% slower
arma/arma.stan 0.65 0.73 0.9 -11.36% slower
garch/garch.stan 0.5 0.56 0.9 -10.74% slower
Mean result: 0.983105752886

Jenkins Console Log
Blue Ocean
Commit hash: 4437d2f


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
Member

@bbbales2 bbbales2 left a comment

Choose a reason for hiding this comment

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

Good!

@bbbales2 bbbales2 changed the title Feature/991 error codes Print errors to stderr instead of stdout Mar 12, 2021
@bbbales2
Copy link
Member

I updated the pull request name so it'd be easier to remember to add a warning about this to the release notes.

@stan-buildbot
Copy link
Contributor


Name Old Result New Result Ratio Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan 3.46 3.42 1.01 1.05% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.02 0.02 1.04 3.72% faster
eight_schools/eight_schools.stan 0.11 0.11 1.0 0.09% faster
gp_regr/gp_regr.stan 0.16 0.16 1.02 1.93% faster
irt_2pl/irt_2pl.stan 5.3 5.33 0.99 -0.55% slower
performance.compilation 90.13 88.6 1.02 1.71% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 8.93 9.11 0.98 -2.04% slower
pkpd/one_comp_mm_elim_abs.stan 29.83 29.7 1.0 0.43% faster
sir/sir.stan 128.45 126.27 1.02 1.7% faster
gp_regr/gen_gp_data.stan 0.04 0.03 1.25 19.91% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan 3.23 3.06 1.05 5.16% faster
pkpd/sim_one_comp_mm_elim_abs.stan 0.4 0.4 1.0 -0.46% slower
arK/arK.stan 1.96 2.63 0.74 -34.31% slower
arma/arma.stan 0.65 0.74 0.88 -13.18% slower
garch/garch.stan 0.5 0.56 0.89 -12.79% slower
Mean result: 0.99310167771

Jenkins Console Log
Blue Ocean
Commit hash: 4437d2f


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 bbbales2 merged commit 7c2adcd into develop Mar 12, 2021
@mitzimorris mitzimorris changed the title Print errors to stderr instead of stdout Print errors to stderr instead of stdout; don't nag for fixed_param flag, just do it Mar 16, 2021
@WardBrian WardBrian deleted the feature/991-error-codes branch September 6, 2023 20:17
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.

4 participants