stan-dev / stan

Stan development repository. The master branch contains the current release. The develop branch contains the latest stable development. See the Developer Process Wiki for details.
https://mc-stan.org
BSD 3-Clause "New" or "Revised" License
2.57k stars 368 forks source link

Issue/3151 laplace logging newlines #3153

Closed mitzimorris closed 1 year ago

mitzimorris commented 1 year ago

Submission Checklist

Summary

Remove spurious whitespace from messages sent to the logger. Added unit test to verify template parameter jacobian.

Intended Effect

Compact console output; messages are 1 per line.

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:

WardBrian commented 1 year ago

While reviewing https://github.com/stan-dev/cmdstan/pull/1134 I noticed the newlines were still logging and realized this PR hadn't been merged yet. @bob-carpenter do you have a chance to finish reviewing the latest changes?

mitzimorris commented 1 year ago

ready to merge when tests pass.

mitzimorris commented 1 year ago

@WardBrian - why are ubuntu checks failing w/ complaint about g++?

WardBrian commented 1 year ago

Better to ask @serban-nicusor-toptal I think. Github did recently change the meaning of ubuntu-latest, but not recently enough that I think this would break it? https://github.blog/changelog/2022-11-09-github-actions-ubuntu-latest-workflows-will-use-ubuntu-22-04/

WardBrian commented 1 year ago

I think it actually is related to the ubuntu-latest changes, not sure clang-6 is available on 22.04 any longer. We can ignore it for this PR I think, I'll ask Nic to take a look