bool is_empty() for not writing to files.

[SteveBronder]

Submission Checklist

• Run unit tests: ./runTests.py src/test/unit
• Run cpplint: make cpplint
• Declare copyright holder and open-source license: see below

Summary

Fixes for #1055 @rok-cesnovar @fcostin I ended up taking the bool approach as when I looked over the code I often see we do things to write to the diagnostics like in mcmc_writer.hpp

  void write_diagnostic_params(stan::mcmc::sample& sample,
                               stan::mcmc::base_mcmc& sampler) {
    std::vector<double> values;

    sample.get_sample_params(values);
    sampler.get_sampler_params(values);
    sampler.get_sampler_diagnostics(values);

    diagnostic_writer_(values);
  }

and I'm like, why do any of that? If we have a bool inside of the writer we can just check if we've reported this thing as empty already and then do nothing in the entire function by just wrapping the body in if (!is_empty()) { ... }.

So to do that I just put a bool member inside of the writers and added a bool argument to the constructor for each one with a default value of false. So all code should work as normal, then up in command.hpp I can just update the constructor for the writer when we see we are writing diagnostics to an empty stream.

Intended Effect

Remove performance loss from writing diagnostics always from #3028

How to Verify

Tests added to check writing with empty_ == true is a no-op

Side Effects

Documentation

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): Steve Bronder

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/)

[rok-cesnovar]

Nice! Thanks Steve!

Can you change so that the PR is done towards master? I think we want to do a .2 release, given our 4 month releae schedule.

[rok-cesnovar]

I will review in a bit.

[SteveBronder]

@rok-cesnovar Do we want it on master or do we want to merge this to develop and then do a full release of 2.28.2? Maybe in the future we could make a branch for each release and then when we do bugfixes we can cherry pick changes and patch them into previous releases? I think that's what Eigen does

https://gitlab.com/libeigen/eigen/-/branches

[rok-cesnovar]

Do we want it on master or do we want to merge this to develop and then do a full release of 2.28.2?

We want it done towards master as we want to release 2.28.1 plus this PR. The current state of develop has other stuff as well, like other features which would mean it would have to be 2.29 if we release that.

Maybe in the future we could make a branch for each release and then when we do bugfixes we can cherry pick changes and patch them into previous releases?

We have release branches as well: https://github.com/stan-dev/stan/tree/release/v2.28.1

If we wanted to back-port some fix to 2.27.0 we should do it to that branch, but given that we are patching the current release I think just doing it to master is the easiest also for release script purposes.

[rok-cesnovar]

And master will be merged to develop then obviously.

[rok-cesnovar]

I tried to work some git magic to make this branch mergeable to master, but that won't fly. Will make a separate branch and open a separate PR with your changes. Thanks again!