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

Bugfix/include what you use in /prim/fun #1711

Merged
merged 12 commits into from
Feb 15, 2020
Merged

Conversation

rok-cesnovar
Copy link
Member

@rok-cesnovar rok-cesnovar commented Feb 14, 2020

Summary

This fixes a bunch of include-what-you-use issues in /prim. There are a lot of functions and files like the ones mentioned in #1704 with missing includes of stan/math/prim/fun/exp.hpp and the rest.
These functions dont have much issues in practice, because we usually include master header files that include all files. But that doesnt mean that this should be cleaned up.

I made a simple Python script that flagged these files which I then fixed.

Example on prim/fun/beta.hpp (comments stripped):

#include <stan/math/prim/meta.hpp>
#include <stan/math/prim/fun/lgamma.hpp>
#include <cmath>

template <typename T1, typename T2>
inline return_type_t<T1, T2> beta(const T1 a, const T2 b) {
  using std::exp;
  return exp(lgamma(a) + lgamma(b) - lgamma(a + b));
} 

There is a missing #include <stan/math/prim/fun/exp.hpp> here which this PR adds. And that is it.
It also does some smaller changes:

  • removes some unnecessary includes that were flagged by the same script
  • removes a few unnecessary local variables that were only used once
  • fix one error message in mdivide_right_tri
  • did a bit of alphabetical ordering of includes but not systematically

Tests

/

Side Effects

/

Copyright holder: Rok Češnovar

The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the 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 rok-cesnovar changed the title Bugfix/include what you use in /prim Bugfix/include what you use in /prim/fun Feb 14, 2020
@stan-buildbot
Copy link
Contributor


Name Old Result New Result Ratio Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan 4.96 4.97 1.0 -0.15% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.02 0.02 1.01 0.63% faster
eight_schools/eight_schools.stan 0.09 0.09 1.01 1.35% faster
gp_regr/gp_regr.stan 0.22 0.22 1.0 0.22% faster
irt_2pl/irt_2pl.stan 6.06 6.05 1.0 0.08% faster
performance.compilation 88.12 87.01 1.01 1.26% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 7.49 7.47 1.0 0.3% faster
pkpd/one_comp_mm_elim_abs.stan 20.53 20.68 0.99 -0.74% slower
sir/sir.stan 93.6 91.09 1.03 2.68% faster
gp_regr/gen_gp_data.stan 0.05 0.05 1.0 0.44% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan 3.01 3.01 1.0 -0.0% slower
pkpd/sim_one_comp_mm_elim_abs.stan 0.32 0.32 0.99 -1.37% slower
arK/arK.stan 1.76 1.74 1.01 0.94% faster
arma/arma.stan 0.8 0.8 1.0 -0.05% slower
garch/garch.stan 0.58 0.59 0.99 -1.48% slower
Mean result: 1.00285317853

Jenkins Console Log
Blue Ocean
Commit hash: 9d94901


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
Contributor

@mcol mcol left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for the little cleanups on locals and header ordering too!

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.

3 participants