remove std::cout and std::cerr from API

bob-carpenter commented 10 years ago

[ ] remove std::cout
[ ] remove std::cerr
[ ] remove std::cin

We can't have std::cout or std::cerr in our API because it doesn't play nicely with uses in clients that control their own streams, like the RStan interface. Interfaces like RStan need to use the R input and output streams instead, or the messages get lost.

The ONLY place we should have the standard I/O streams is in CmdStan.

If we need to pass info about errors or other output, the streams must be plumbed down from the top-level application.

bob-carpenter commented 10 years ago

Suggestion from Ben Goodrich:

[ ] add a git hook to the Jenkins machine that git greps for std::cout and std::cerr in the commits before running the tests on a pull request.

bob-carpenter commented 10 years ago

Files to fix:

[x] io/dump.hpp: add ostream& arg to print() and remove std::cout and update doxygen
[ ] io/mcmc_writer.hpp: add ostream& arg to print_timing() and pass in &std::cout from caller in gm/command.hpp
[ ] io/stan_csv_reader.hpp: add ostream& arg to read_samples() and parse() and pass in &std::cout from command/print.cpp
[x] mcmc/chains.hpp: add ostream& arg to covariance() and correlation() and figure out who's calling them and pass it in OR remove warning message to std::cerr OR throw an exception. These are mismatched size tests and we can't handle mismatched sizes properly anyway (the code just takes the min).
[x] mcmc/hmc/nuts/adapt_dense_e_nuts.hpp: elimiate default &std::cout arg and either make the default 0 OR don't have a default (in second case, bonus points for replacing pointer with non-const reference)
[x] mcmc/hmc/nuts/adapt_diag_e_nuts.hpp: ditto
[x] hmc/nuts/adapt_unit_e_nuts.hpp: again
[x] hmc/nuts/dense_e_nuts.hpp: once again
[x] hmc/nuts/unit_e_nuts.hpp: keep going
[x] hmc/static/adapt_dense_e_static_hmc.hpp: almost there
[x] hmc/static/adapt_diag_e_static_hmc.hpp: just kidding
[x] hmc/static/adapt_unit_e_static_hmc.hpp: script, anyone?
[x] hmc/static/dense_e_static_hmc.hpp: light at the end of the tunnel
[x] hmc/static/diag_e_static_hmc.hpp: penultimate
[x] hmc/static/unit_e_static_hmc.hpp: end of the tunnel
[x] model/util.hpp: remove default, which means msgs can't get a default (could make a pointer and default to 0 if handled properly)
[ ] finally, make sure to remove any includes of iostream

syclik commented 10 years ago

I'm working on it under Stan's feature/issue-382-remove-std-cout-cerr, in case anyone else wants to help out.

syclik commented 10 years ago

New list:

[ ] src/stan/command/print.cpp
[x] src/stan/command/print.hpp
[x] src/stan/command/stanc.cpp
[x] src/stan/command/stanc_helper.cpp
[ ] src/stan/common/command.hpp
[x] src/stan/common/init_adapt.hpp
[x] src/stan/common/init_windowed_adapt.hpp
[x] src/stan/common/write_iteration.hpp
[x] src/stan/gm/grammars/program_grammar_def.hpp
[ ] src/stan/gm/generator.hpp
[x] src/stan/io/stan_csv_reader.hpp

bob-carpenter commented 10 years ago

The one in generator.hpp (not generators.hpp) is going to be tricky because it's quoted in generated code. We need to have a way to grab an output stream to pass in.

The program_grammar_def.hpp is a stray commented out line that should just be deleted.

Bob

On May 19, 2014, at 5:52 PM, Daniel Lee notifications@github.com wrote:

New list:

• src/stan/command/print.cpp • src/stan/command/print.hpp • src/stan/command/stanc.cpp • src/stan/command/stanc_helper.cpp • src/stan/common/command.hpp • src/stan/common/init_adapt.hpp • src/stan/common/init_windowed_adapt.hpp • src/stan/common/write_iteration.hpp • src/stan/gm/grammars/program_grammar_def.hpp • src/stan/gm/generators.hpp • src/stan/io/stan_csv_reader.hpp — Reply to this email directly or view it on GitHub.

syclik commented 10 years ago

Thanks for the heads up.

If the generator.hpp is too complicated, I'll leave it and add another issue for the remaining one.

On Mon, May 19, 2014 at 2:15 PM, Bob Carpenter notifications@github.comwrote:

The one in generator.hpp (not generators.hpp) is going to be tricky because it's quoted in generated code. We need to have a way to grab an output stream to pass in.

The program_grammar_def.hpp is a stray commented out line that should just be deleted.

Bob

On May 19, 2014, at 5:52 PM, Daniel Lee notifications@github.com wrote:

New list:

• src/stan/command/print.cpp • src/stan/command/print.hpp • src/stan/command/stanc.cpp • src/stan/command/stanc_helper.cpp • src/stan/common/command.hpp • src/stan/common/init_adapt.hpp • src/stan/common/init_windowed_adapt.hpp • src/stan/common/write_iteration.hpp • src/stan/gm/grammars/program_grammar_def.hpp • src/stan/gm/generators.hpp • src/stan/io/stan_csv_reader.hpp — Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHubhttps://github.com/stan-dev/stan/issues/382#issuecomment-43537859 .

syclik commented 10 years ago

refactoring src/stan/gm/generator.hpp and src/stan/common/command.hpp are beyond the scope of a single pull request, so I'm creating a new issue.

syclik commented 10 years ago

Based on v2.3.0.

source with std::cout

[ ] src/stan/command/print.cpp
[ ] src/stan/command/print.hpp
[ ] src/stan/command/stanc.cpp
[ ] src/stan/common/command.hpp
[ ] src/stan/common/init_adapt.hpp
[ ] src/stan/common/init_windowed_adapt.hpp
[ ] src/stan/common/write_iteration.hpp
[ ] src/stan/gm/grammars/program_grammar_def.hpp
[ ] src/stan/io/dump.hpp
[ ] src/stan/io/stan_csv_reader.hpp
[ ] src/stan/mcmc/fixed_param_sampler.hpp
[ ] src/stan/mcmc/hmc/nuts/adapt_dense_e_nuts.hpp
[ ] src/stan/mcmc/hmc/nuts/adapt_diag_e_nuts.hpp
[ ] src/stan/mcmc/hmc/nuts/adapt_unit_e_nuts.hpp
[ ] src/stan/mcmc/hmc/nuts/dense_e_nuts.hpp
[ ] src/stan/mcmc/hmc/nuts/diag_e_nuts.hpp
[ ] src/stan/mcmc/hmc/nuts/unit_e_nuts.hpp
[ ] src/stan/mcmc/hmc/static/adapt_dense_e_static_hmc.hpp
[ ] src/stan/mcmc/hmc/static/adapt_diag_e_static_hmc.hpp
[ ] src/stan/mcmc/hmc/static/adapt_unit_e_static_hmc.hpp
[ ] src/stan/mcmc/hmc/static/dense_e_static_hmc.hpp
[ ] src/stan/mcmc/hmc/static/diag_e_static_hmc.hpp
[ ] src/stan/mcmc/hmc/static/unit_e_static_hmc.hpp
[ ] src/stan/model/util.hpp

source with std::cerr

[ ] src/stan/command/stanc.cpp
[ ] src/stan/command/stanc_helper.hpp
[ ] src/stan/common/command.hpp
[ ] src/stan/gm/generator.hpp
[ ] src/stan/mcmc/chains.hpp

test with std::cout

[ ] src/test/unit/common/command_init_test.cpp
[ ] src/test/unit/common/print_progress_test.cpp
[ ] src/test/unit/common/run_markov_chain_test.cpp
[ ] src/test/unit/common/sample_test.cpp
[ ] src/test/unit/common/warmup_test.cpp
[ ] src/test/unit/common/write_iteration_test.cpp
[ ] src/test/unit/gm/parser_test.cpp
[ ] src/test/unit/io/dump_test.cpp
[ ] src/test/unit/io/json/json_data_test.cpp
[ ] src/test/unit/io/json/json_parser_test.cpp
[ ] src/test/unit/io/mcmc_writer_test.cpp
[ ] src/test/unit/math/functions/lgamma_test.cpp
[ ] src/test/unit/mcmc/hmc/base_hmc_test.cpp
[ ] src/test/unit/mcmc/hmc/base_nuts_test.cpp
[ ] src/test/unit/mcmc/hmc/base_static_hmc_test.cpp
[ ] src/test/unit/mcmc/hmc/hamiltonians/base_hamiltonian_test.cpp
[ ] src/test/unit/mcmc/hmc/hamiltonians/dense_e_metric_test.cpp
[ ] src/test/unit/mcmc/hmc/hamiltonians/diag_e_metric_test.cpp
[ ] src/test/unit/mcmc/hmc/hamiltonians/unit_e_metric_test.cpp
[ ] src/test/unit/mcmc/hmc/integrators/expl_leapfrog2_test.cpp
[ ] src/test/unit/mcmc/hmc/integrators/expl_leapfrog_test.cpp
[ ] src/test/unit/model/util_test.cpp
[ ] src/test/unit/prob/autocorrelation_test.cpp
[ ] src/test/unit/prob/autocovariance_test.cpp
[ ] src/test/unit/prob/distributions/multivariate/continuous/inv_wishart_test.cpp
[ ] src/test/unit/prob/distributions/univariate/continuous/normal_test.cpp
[ ] src/test/unit/prob/transform_test.cpp

test with std::cerr

[ ] src/test/unit/mcmc/hmc/base_hmc_test.cpp
[ ] src/test/unit/mcmc/hmc/base_nuts_test.cpp
[ ] src/test/unit/mcmc/hmc/base_static_hmc_test.cpp

syclik commented 10 years ago

I'm working on this again on branch: feature/issue-382-remove-cout-cerr

stan-dev / stan